threads and Window.dispose

Thread view:

John Harrison - 29 Mar 2004 11:52 GMT

I'm working on a large java app which was written by someone else. I'm also
a java newbie.

My problem is that the application hangs on Mac OS X with the 1.4.2
platform, it did not hang on 1.3.1 and it does not hang on Windows.

It seems to be a thread deadlock situation and the cause is code like the
following

class SomeWindow extends Window
{
...
public void dispose()
{
synchronized (this)
{
// some app specific code
...
super.dispose();
}
}
...
}

The problem goes away if I remove the synchronized keyword, it also goes
away if I remove the call to super.dispose() from inside the synchronized
clause.

So my question is whether code like the above is legitimate, or is there
something wrong with disposing of a Window while synchronizing on it.

Thanks,
John

Reply to this Message

Tony Morris - 29 Mar 2004 12:52 GMT

> I'm working on a large java app which was written by someone else. I'm also
> a java newbie.
[quoted text clipped - 29 lines]
> Thanks,
> John

Two very important things to understand:
First, when dealing with threads, behaviour may be indeterminate, and
therefore, does not occur all the time. For this reason, it is best if you
produce a compilable, accurate test case that demonstrates the problem,
along with a detailed explanation of the environment in which the problem
occurs. i.e. I could very well put that code on my (imaginary) Mac OS
machine and the problem does not occur.

Second, dealing with threads is a relatively complicated task, given that
you do not have a working knowledge of Java. There are more "gotchyas" that
many people care to learn about. Perhaps you might want to read up:
http://java.sun.com/docs/books/tutorial/essential/threads/index.html

For more advanced reading on threads in Java, "Taming Java Threads", by
Allen Holub is one of my favourites, although, I've heard criticism from
very reputable persons regarding this book (I can't help but think that
these opinions are bias because the reputable persons are on the Sun Java
development team, and Holub is very critical of the Java threading model).

Tony Morris
(BInfTech, Cert 3 I.T., SCJP[1.4], SCJD)
Software Engineer
IBM Australia - Tivoli Security Software
(2003 VTR1000F)

Reply to this Message

Babu Kalakrishnan - 29 Mar 2004 13:35 GMT

> I'm working on a large java app which was written by someone else. I'm also
> a java newbie.
[quoted text clipped - 26 lines]
> So my question is whether code like the above is legitimate, or is there
> something wrong with disposing of a Window while synchronizing on it.

The general rule of thumb to be followed while synchronizing to any object is
that you should have an idea as to which other code is synchronized to the
same object. This is essential for building a deadlock free system.

In my experience, it is inherently unsafe to use any object in the java.awt or
javax.swing hierarchy for synchronization.This is basically because the awt
subsystem is not threadsafe, and there is absolutely no documentation on which
parts of Sun's code is synchronized to which object.

Therefore for achieving your synchronization requirements, it is always best to
create a separate object to lock on (instead of synchronizing on "this").

Of course there are some cases where you *need* to synchronize with the "this"
object (or on an object of a class that belongs to the java library), because
you want to depend on the thread safety of inherited methods in that class.
In such a case you should ensure that you know exactly how the documented
synchronization behaviour of this class is. There are several parts of the
java library with reasonably well defined synchronization behaviour (such as
the Collections classes), on which you can depend on - but the awt / swing
classes certainly don't fall into that category.

BK

Reply to this Message

John Harrison - 29 Mar 2004 13:50 GMT

> > I'm working on a large java app which was written by someone else. I'm also
> > a java newbie.
[quoted text clipped - 49 lines]
>
> BK

Thanks for the info. Your argument that you need to know what other code is
synchronized on the same object and that this is not documented for
Swing/AWT classes certainly makes sense to me.

The other thing I've noticed is that this deadlock only occurs when there
are no other windows on the screen. This occurs in two situations, removal
of the splash screen and removal of the final window on program exit. As you
say its probably the internal details of the implementation that are
relevant here.

As I said I inherited this code and I'm currently trying to find out why
synchronize(this) appears in the dispose method (it does so consistently
throughout the code). It entirely possible that there is no good reason for
this synchronization at all, certainly none leaps out when looking at the
code.

In my (possibly naive) view this would seem much more reasonable.

class SomeWindow extends Window
{
public void dispose()
{
synchronized (this)
{
// app specific code
...
}
super.dispose();
}

in other words if the super class needs to synchronize anything, it can do
it itself. There's no threading or eventing within the app specific code,
just some simple cleanup. Do that seem more reasonable to you?

john

Reply to this Message

Babu Kalakrishnan - 29 Mar 2004 14:39 GMT

> In my (possibly naive) view this would seem much more reasonable.
>
[quoted text clipped - 13 lines]
> it itself. There's no threading or eventing within the app specific code,
> just some simple cleanup. Do that seem more reasonable to you?

Even this can (I'm not saying it will) cause deadlocks because the dispose()
method can (theoretically) be called by some other code within the AWT expecting
that there is no synchronized block within the dispose call.

The ideal thing to do would be to locate within the class all blocks of code
that is synchronized to "this" (that will include synchronized(this) blocks
and instance methods that are synchronized) and change them to synchronize
on a lock object created fro this purpose. Also, if any other classes refer
to objects of this type and do synchronization on an instance of the class,
they would need to be modified to synchronize on the lock object as well.

For example :

In YourDialog.java

private Object lockObject = new Object();

synchronized void someMethod ()
{
    // Code
}

would become :

void someMethod()
{
    synchronized(lockObject)
    {
        // Code
    }
}

and

synchronized(this)
{
}

would become :

synchronized(lockObject)
{
}

Also external classes which have code like :

synchronized(yourDialogInstance)
{
}

would need to be changed to

synchronized(yourDialogInstance.getLockObject())
{
}

(you need to provide an accessor as well in such a case).

BK

Reply to this Message

John Harrison - 30 Mar 2004 10:31 GMT

> > In my (possibly naive) view this would seem much more reasonable.
> >
[quoted text clipped - 24 lines]
> to objects of this type and do synchronization on an instance of the class,
> they would need to be modified to synchronize on the lock object as well.

Thanks for the advice, I'll go with your suggestion of an separate lock
object.

john

Reply to this Message

SPG - 29 Mar 2004 13:36 GMT

Hi,

Try putting the super.dispose() call outside of the sync block.
That may help..

> I'm working on a large java app which was written by someone else. I'm also
> a java newbie.
[quoted text clipped - 29 lines]
> Thanks,
> John

Reply to this Message

John Harrison - 29 Mar 2004 13:50 GMT

> Hi,
>
> Try putting the super.dispose() call outside of the sync block.
> That may help..

It does indeed. Thanks, John

Reply to this Message

threads and Window.dispose

Free Magazines