views:

467

answers:

7

Hello

When you want a certain task to be executed by another thread, you can extend Thread or implement Runnable.

I've made an attempt to create a class which runs a class entirely in the second thread.

This means that you can call anyMethod() which returns immediately and which is executed by the second thread.

Here is my attempt:

import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;

/**
 * Extend this class to run method calls asynchronously in the second thread implemented by this class.
 * Create method(type1 param1, type2 param2, ...) and let it call this.enqueueVoidCall("method", param1, param2, ...)
 * 
 * The thread executing the run-method will automatically call methodAsync with the specified parameters.
 * To obtain the return-value, pass an implementation of AsyncCallback to this.enqueueCall().
 * AsyncCallback.returnValue() will automatically be called upon completion of the methodAsync.
 *  
 */
public class ThreadedClass extends Thread {
    private static Object test;

    private Queue<String> queue_methods = new ConcurrentLinkedQueue<String>();
    private Queue<Object[]> queue_params = new ConcurrentLinkedQueue<Object[]>();
    private Queue<AsyncCallback<? extends Object>> queue_callback = new ConcurrentLinkedQueue<AsyncCallback<? extends Object>>();

    private volatile boolean shutdown = false;

/**
 * The run method required by Runnable. It manages the asynchronous calls placed to this class.
 */
@Override
public final void run() {
 test = new Object();
 while (!shutdown) {
  if (!this.queue_methods.isEmpty()) {
   String crtMethod = queue_methods.poll();
   Object[] crtParamArr = queue_params.poll();
   String methodName = crtMethod + "Async";

   Method method;
   try {
    method = this.getClass().getMethod(methodName);
    try {
     Object retVal = method.invoke(this, crtParamArr);
     AsyncCallback<? extends Object> crtCallback = queue_callback.poll();
     crtCallback.returnValue(retVal);
    } catch (Exception ex) {}
         } catch (SecurityException ex) {
               } catch (NoSuchMethodException ex) {}
  } else {
   try {
    synchronized(test ) {
     test.wait();
    }
   } catch (InterruptedException ex) {
    System.out.println("READY");
   } catch (Exception ex) {
    System.out.println("READY, but " + ex.getMessage());
   }
  }
 }
}

/**
 * Asynchronously adds a method-call to the scheduler, specified by methodName with passed parameters 
 * @param methodName The name of the currently called method. methodName + "Async" is being called
 * @param parameters Parameters you may want to pass to the method
 */
protected final void enqueueVoidCall(String methodName, Object... parameters) {
 List<Object> tmpParam = new ArrayList<Object>();
 for (Object crt : parameters) {
  tmpParam.add(crt);
 }
 queue_methods.add(methodName);
 queue_params.add(parameters);
 queue_callback.add(null);
 test.notifyAll();
}

/**
 * Asynchronously adds a method-call to the scheduler, specified by methodName with passed parameters 
 * @param methodName The name of the currently called method. methodName + "Async" is being called
 * @param callBack An instance of AsyncCallback whose returnValue-method is called upon completion of the task.
 * @param parameters Parameters you may want to pass to the method
 */
protected final void enqueueCall(String methodName, AsyncCallback<? extends Object> callBack, Object... parameters) {
 List<Object> tmpParam = new ArrayList<Object>();
 for (Object crt : parameters) {
  tmpParam.add(crt);
 }
 queue_methods.add(methodName);
 queue_params.add(parameters);
 queue_callback.add(callBack);
 test.notifyAll();
}

/**
 * Complete the currently running task, optionally return values and eventually shut down. The instance of this object becomes unusable after this call. 
 */
public void shutdown() {
 shutdown=true;
}

}

Now i have two classes to test things:

public class MySecondTask extends ThreadedClass {
public void test1() {
 this.enqueueVoidCall("test1", null);
}

public void test1Async() {
 System.out.println("Start");
 try {
  // do big job here
 } catch (Exception ex) { }
 System.out.println("Done");
}
}

And the main-method starting the stuff:

public class TestingClass {
public static void main(String[] args) {
 MySecondTask test = new MySecondTask();
 test.start();
 System.out.println("1. Thread [1]");
 // CORRECTION, SHOULD BE:
 test.test1();
 // INSTEAD OF:
 // test.test1Async();
 for(int q=0; q<=100000; q++) {
  System.out.println("1:"+ new Date().getTime()+":"+ q);
  if ((q % 1000) == 0) {
   System.out.flush();
  }
 }
 System.err.println("1. Thread [2]");
}

}

Somehow, the output of the second thread always appears first (entirely), and then the rest puts out on the console. If the threads were running concurrently (which is the intended result), the console-output should be mixed?!

Any idea is appreciated as well as comments to improve my coding style.


EDIT:

The problem cited is quite solved.

Now I receive an IllegalMonitorStateException on the line where i call: ThreadedClass.notifyAll().

Maybe i got that one with the lock wrong.

But a) why is it required to use synchronized() around wait() and how can i make the notifyAll()-call to unblock wait()?


thanks in advance and best regards

p.s.: you all do a good job on stack overflow. you already helped me many times without knowing it, thanks for that. keep it up!

A: 

Your main thread waits for test.test1Async() to return, what do you do in there?

CookieOfFortune
A: 

Java threads run differently on different machines. Some machines are preemptive others are not. If the second thread outputs its contents before the first, then the machine your code is running on is most likely non-preemptive. If you need the Threads to be synchronized there are ways of doing this, but if you do not care about synchronization, then there is no guarantee as to how your Threads will run.

Also, test.test1Async() is running in the first thread, not the second. (This may be what is holding things up)

SquareRootOf2
okay i can understand the first point.but it should not be running in the first thread, because the thread which is currently in the run-method should call it (that's my intention).why is it being called from the first thread? what's wrong about that...
Atmocreations
Okay, what OS that is capable of reading StackOverflow and running a JVM today is NOT preemptively multithreaded?
Chris Kaminski
@Atmocreations - When you tell the second thread to start it begins its execution in the run method, but test.test1Async() is not called in the run method, its called in the main method of the main thread. Try not even starting the second Thread, test1Async() will still run.
SquareRootOf2
+5  A: 

This means that you can call anyMethod() which returns immediately and which is executed by the second thread.

This sounds suspiciously like working with callables, futures and executors:

Hate to break it to you, but you might really want to look into this stuff..

Edit to address comment below

Just make your methods in your object look like this:

private ExecutorService executorService = Executors.newCachedThreadPool();

public Future<SomeObject> yourMethodName(String anyArguments) {
    return executorService.submit(
     new Callable<SomeObject>() {
      public SomeObject call() {
       SomeObject obj = new SomeObject();
       /* Your Code Here*/;
       return obj;
      }
     }
    );
}
Tim
thanks for the tip. i already heard of them and i think i know what these facilities do.my problem with those is that i would need to create another implementation of Runnable for _each_ of the methods of a class i want to call. or am i getting it wrong?I would just like to be able to call myObject.method1(), maybe later myObject.method2(). These need to return immediately and do the background work asynchronously. I thought with my attempt using a "call-queue", this task could be done...?!
Atmocreations
Take a look at the new code block above.. This type of implementation should really be enough to solve most problems. If you want to make it more scary / complicated you could add reflection to add the Callables and threadpool submittal for you, but I wouldn't recommend it.
Tim
thanks. in my opinion, this looks more complicated to me as the one i did. but i will take a closer look at it to see if it could help me.
Atmocreations
I agree with Tim, this is the best way to do it unless you want to go into AOP. See my answer below for why your approach while technically correct is not desirable.
Gregory Mostizky
looks as complicated as clean. maybe i just got to take a deep look at things like Executors etc. Does anybody know a _good_ tutorial about how these exactly work? i found something in javadoc of the classes/interfaces, but maybe there are some good documentation you can recommend?
Atmocreations
I've added a few links to the post that have some explanation in the Javadocs.. It's hard to find a good tutorial about this stuff as it's mostly still old tutorials working with Threads.. You should be able to figure it out from the Javadocs though, and maybe take a look at my answer here for a working example: http://stackoverflow.com/questions/1049737/are-tasks-parallelized-when-executed-via-an-executorcompletionservice/1053530#1053530
Tim
A: 

Style suggestion: look at java.lang.reflect.Proxy and InvocationHandler. You could implement an InvocationHandler to avoid some of the reflection stuff you're dealing with, and users can call the real Interface methods directly.

A: 

You are calling test1Async Sychronously. If your //bigjob is done here, then it will not run in parallel to your other threads. What you want to do is this:

public class TestingClass {
public static void main(String[] args)
{
    MySecondTask test = new MySecondTask();
    test.start();
    System.out.println("1. Thread [1]");
    //test.test1Async();
    for(int q=0; q<=100000; q++)
    {
            System.out.println("1:"+ new Date().getTime()+":"+ q);
            if ((q % 1000) == 0)
            {
                    System.out.flush();
            }
    }
    System.err.println("1. Thread [2]");
}

}

Remember, your ThreadedClass.run() method is going to call test1.testAsynch() for you. I'm actually surprised you're not seeing the result output twice, or messed up calculations.

Chris Kaminski
seen your point, thx. got another problem now... (you may want to see my comment on oscar reyes answer)
Atmocreations
+2  A: 

You're never calling your "threaded" dispatch mechanism ( the one that uses the queue etc )

I guess you attempt to call:

test.test1();

Which in turn enqueue the call to test1Async, but by mistake you called:

test.test1Async();

Directly making the whole execution in a single thread.

Replace:

    ....
    System.out.println("1. Thread [1]");
    test.test1Async();
    for(int q=0; q<=100000; q++)
    {
    ...

With:

   ....
    System.out.println("1. Thread [1]");
    test.test1();
    for ( int q=0; q<=100000 ; q++ ) {
    ....

On the coding style, pleeeeease use the opening brace in the same line as the statement when coding in Java ( and JavaScript ) In C# , C++ and C is better as the way you have it.

Also use camelCase instead of separete_with_underscore.

Here's a document with more about Java's coding conventions.

OscarRyz
ouch, yes you're right. (where did my comment i just posted go to?)as darthcoder commented out the line i saw that something can't be right.after some other little correction i managed to run the code again. but this time, it throws IllegalMonitorStateException on the line where i call notifyAll(). Sure the 2nd thread (which is now calling that method) isn't the lock-owner, i know that. But that's what notify is for, right? to continue other threads waiting for this. any idea how to correct it?thanks
Atmocreations
From the JDK Reference: [IllegalMonitorStateException - if the current thread is not the owner of this object's monitor.] The problem is that the thread that calls notifyAll() never synchronizes() on test.
Chris Kaminski
If you don't post the stacktrace is very hard to "guess", but I'll guess. In method: enqueueCall() you call test.notifyAll(); In order to use notify all the thread must own the object lock.
OscarRyz
So, add synchonized( test ) { test.notifyAll(); } and see what happens. BTW I don't know much about the new concurrency framework, but I'm pretty sure it should be used to about this kind of complexities witht he notify/synchronize stuff. I know how to do it using bare locks/synchronize. Let me now if you need more help on this. If so, please consider clean up your code and put all the opening braces in one line ( it is hard to follow for me the way they are )
OscarRyz
+1  A: 

I suggest you implement it in exactly the way Tim suggested in his answer.

Your own idea, while inventive, breaks a lot of best practices which make your code very brittle. There is just to many things to remember otherwise it will subtly break.

Just think of the guy that will come after you and have to use, maintain and extend your code. Think what will happen when you need to revisit this code in a year.

Just a short list of stuff you shouldn't do:

  • Extending Thread directly is considered bad practice, prefer to implement Runnable instead
  • Avoid encoding methods as text - it will break on first refactoring
  • test1Async() should be private otherwise someone new to the team will call it directly
  • Method naming should be clear - *Async() usually means do in the background while it's actually the other way around
  • Overall brittleness - let's say I need to change test1() to return int instead of void - will I really rememeber to change the other method as well? Will you remember to do it a year later?
Gregory Mostizky
thx for your response.okay, implementing Runnable instead of extending Thread is something i could easily live with...making ...Async() private (or protected for extensibility) is a good point as well. for now i didn't really care about that because i just wanted the code to run as expected ;o)maybe it's brittled, i agree. but when i implement it the way Tim suggested, i guess i would have the same problem again with the remembering of the different changes i would have to do when changing the code.
Atmocreations
You have a good positive attitude which is very cool :) Regarding Tim's solution - the advantage is that it's very concentrated. So for example if you want to change some method you go to the method and change it - and that's it. No need to go to another method, no need to update some text string somewhere - just find the method and fix it. This might not seem important now, but in a few months you will thank me :)
Gregory Mostizky