Looks like it should work in theory, though it doesn't look it acquires/holds the lock during any useful operations other than to use wait/notify. This might work because you may not have any need for any other synchronization.
For example, take a look at my run method above:
public synchronized void run()
{
while(keep_running)
{
wait(); // wait for someone to give us a task or tell us to stop
if(keep_running && task != null)
{
task.run();
}
}
}
notice that my program holds onto the lock while it's running the task. This means that any other thread which is trying to run a task with this worker will wait until it can acquire the lock, guaranteeing that all tasks get run. On the other hand, say I try doing this:
public void run()
{
while(keep_running)
{
synchronized(this)
{
wait(); // wait for someone to give us a task or tell us to stop
}
if(keep_running && task != null)
{
task.run();
}
}
}
This means any other thread which wants the Worker to perform a task will always be able to modify the task variable, and with multiple tasks being assigned it's not even guaranteed that all tasks will run. Additionally the notify() might not even have any effect since my run method must be in a wait() state before notify() is called. These are race conditions which are highly dependent on the timing of what code is executed, which are extremely difficult to debug since the behavior is dependent on timing. For example, I once worked on a project where the code had a race condition. It so happened to be that when I ran the program in debug mode it worked exactly as we wanted it to. However, when run in release mode the timing was just different enough that the program would do things in all the wrong order and fail to function properly.