Synchronized Not Working

[cppaddict]

The main event handling method in my EventHandler class looks like
this:

<code>
public synchronized void performAction(Event e) {
System.out.println(((ActionEvent) e).toString() );
mProcessingStrategy.doAction((ActionEvent) e);
}
</code>

The initial processing strategy's doAction() calls a series of other
methods, some of which are methods of EventHandler's members. While
doAction() is executing a loop in one of these member methods, it
seems that another thread is able to execute the synchronized
performAction(), even though the original thead has not yet returned
and is not wait() ing.

How do I know it hasn't returned?

I put a a println() statement in the loop, and also put a print
statement at the top of performAction, as you can see above. Then I
get output like this:

looping...
looping...
looping...
Printing another ActionEvent....
looping....
looping....

Since mProcessingStrategy's initial doAction() method changes
mProcessingStrategy to a new strategy which does not call the above
loop, I know that the first thread is printing the "looping..." lines
both before and after the new ActionEvent is printed. Yet according to
my understanding of synchronized, this should be impossible. Am I
missing something? What could be allowing this to happen?

Thanks,
cpp

 

[Matt Humphrey]

The main event handling method in my EventHandler class looks like
this:

<code>
public synchronized void performAction(Event e) {
System.out.println(((ActionEvent) e).toString() );
mProcessingStrategy.doAction((ActionEvent) e);
}
</code>

The initial processing strategy's doAction() calls a series of other
methods, some of which are methods of EventHandler's members. While
doAction() is executing a loop in one of these member methods, it
seems that another thread is able to execute the synchronized
performAction(), even though the original thead has not yet returned
and is not wait() ing.

How do I know it hasn't returned?

I put a a println() statement in the loop, and also put a print
statement at the top of performAction, as you can see above. Then I
get output like this:

looping...
looping...
looping...
Printing another ActionEvent....
looping....
looping....

Since mProcessingStrategy's initial doAction() method changes
mProcessingStrategy to a new strategy which does not call the above
loop, I know that the first thread is printing the "looping..." lines
both before and after the new ActionEvent is printed. Yet according to
my understanding of synchronized, this should be impossible. Am I
missing something? What could be allowing this to happen?

Many programs are built on the correctness of the synchronized statement, so
I would think there's something else going on. There's not really enough
information to tell from your description, though. Can you reduce the
problem to a small program and post it? Off the top of my head, I would
check that
1) The performAction is being perform on exactly the same object. It's the
object that is locked, not the method.
2) That the print statement refers to what you think it refers to.
3) That you are sure that same thread (the UI thread?) is not invoking
performAction again as it already owns the lock.

Cheers,
Matt Humphrey (e-mail address removed) http://www.iviz.com/

 

[Gordon Beaton]

The main event handling method in my EventHandler class looks like
this:

<code>
public synchronized void performAction(Event e) {
System.out.println(((ActionEvent) e).toString() );
mProcessingStrategy.doAction((ActionEvent) e);
}
</code>

The initial processing strategy's doAction() calls a series of other
methods, some of which are methods of EventHandler's members. While
doAction() is executing a loop in one of these member methods, it
seems that another thread is able to execute the synchronized
performAction(), even though the original thead has not yet returned
and is not wait() ing.

The methods must be synchronized on the _same_ object. Are they? Think
about the data that needs protecting, not the actions.

If you have declared a method synchronized, the implied object is
"this", which will prevent multiple threads from invoking the methods
on the given object instance ("this" one), while still allowing the
same methods to be invoked on different instances.

If you need to prevent threads from invoking the methods on *any* of
the object instances, define a synchronized block and specify what
object to synchronize on, then make sure all threads use the same
synchronization object.

/gordon

 

[Tony Morris]

Is the method called on the same instance (which acts as the mutex) ?
If not, produce a better test case that demonstrates your misunderstanding.
As it stands, your description is potentially erroneous. Since the basis for
your claim is blatantly incorrect ('synchronized' not working), it's
difficult to trust your description of the scenario -- complete, compilable
source code might help point out your problem.