File APIs for Java Developers
Manipulate DOC, XLS, PPT, PDF and many others from your application.
http://aspose.com/file-tools
The moose likes Java in General and the fly likes Unexpected thread behavior Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


JavaRanch » Java Forums » Java » Java in General
Bookmark "Unexpected thread behavior" Watch "Unexpected thread behavior" New topic
Author

Unexpected thread behavior

Michael Morris
Ranch Hand

Joined: Jan 30, 2002
Posts: 3451
I noticed this thread the other day in the Programmer Certification forum and was shocked when I adapted the code. Take a look at this:

This code does not deadlock as I would have expected. It will however if the line dt.start(); is moved up before any of the other threads are started. This seems to be contradictory to JVM Spec 8.14 which states:
(Statement after mentioning thread T calling wait)
The thread T then lies dormant until one of three things happens:
  • Some other thread invokes the notify method for that object, and thread T happens to be the one arbitrarily chosen as the one to notify.
  • Some other thread invokes the notifyAll method for that object.
  • If the call by thread T to the wait method specified a time-out interval, then the specified amount of real time elapses.

  • This is mimiced in JLS Spec 17.14
    So what am I missing here? Is this a bug in Java HotSpot(TM) Client VM?
    The apparent behavior is that when dt runs and finishes is tantamount to calling notifyAll. Maybe there is something about the Thread class that I do not understand, HELP!
    [ March 09, 2004: Message edited by: Michael Morris ]

    Any intelligent fool can make things bigger, more complex, and more violent. It takes a touch of genius - and a lot of courage - to move in the opposite direction. - Ernst F. Schumacher
    Warren Dew
    blacksmith
    Ranch Hand

    Joined: Mar 04, 2004
    Posts: 1332
        
        2
    Can you be more specific about what behavior and output you are getting, and what behavior you expect?
    I would not expect a deadlock, since the wait() method relinquishes the lock. However, I guess I'm a little surprised if those three threads continue execution beyond the call to dt.wait().
    Michael Morris
    Ranch Hand

    Joined: Jan 30, 2002
    Posts: 3451
    ... However, I guess I'm a little surprised if those three threads continue execution beyond the call to dt.wait().
    That is exactly what they do unless, as I mentioned, the dt thread starts before the other three, in which case a deadlock occurs as the other three threads go into dt's wait set and never come out, as would be expected regardless of the dt thread running.
    Mr. C Lamont Gilbert
    Ranch Hand

    Joined: Oct 05, 2001
    Posts: 1170

    Why should this program deadlock? If you set them as non-daemon threads perhaps the JVM will not exit, but as it stands, the main thread will exit regardless to what those 3 sub threads do.
    and the dt thread is meaningless. It matters not wether its run. Nobody calls notify, so those threads will all wait indefinitely.
    Jim Yingst
    Wanderer
    Sheriff

    Joined: Jan 30, 2000
    Posts: 18671
    Nobody calls notify, so those threads will all wait indefinitely.
    I agree with you CL, that's what should happen here. And htat's what MM was expecting too. But I ran the code, and observed results similar to MM's. About 60% of the time, all three threads block on "waiting..." as expected. 35% of the time though, all three threads complete with "Got lock". Less than 5% of the time, one or two threads wait, while remaining threads complete. For what it's worth, I'm running J2SDK 1.4.2_03 on XP Pro.
    I agree with MM's statement that it seems as if when the thread completes, notifyAll() is called on the Thread object. On the one hand, this seems to contradict JVMS 8.14 as MM noted. The only way I can see around it is to say that each thread does wait() until notifyAll() is called. Nothing in the specs says that something in the JVM can't call notifyAll(). It's undocumented and unexpected, but not stictly speaking illegal. Maybe. Seems pretty iffy though.
    I recall Marlene Miller found a reference somewhere on "spontaneous thread notification" - I recall it sounded deeply suspicious to me at the time, in violation of specs. But maybe there was something to it. Can't find it now in searches...


    "I'm not back." - Bill Harding, Twister
    Ernest Friedman-Hill
    author and iconoclast
    Marshal

    Joined: Jul 08, 2003
    Posts: 24187
        
      34

    I thought this was fairly well-known. Look, for example, at the implementation of Thread.join() in JDK 1.4.2:

    See? All join() does is wait to be notified at a time when isAlive() returns false. This routine wouldn't work unless every thread object were, in fact, reliably signalled when the thread terminated.
    The corrolary to this: don't synchronize on, wait on, or notify Thread objects, unless you understand this and what you're getting into.
    [ March 09, 2004: Message edited by: Ernest Friedman-Hill ]

    [Jess in Action][AskingGoodQuestions]
    Mr. C Lamont Gilbert
    Ranch Hand

    Joined: Oct 05, 2001
    Posts: 1170

    THAT IS FOUL
    They should be ashamed of themselves. There is a difference between a thread object and a thread of execution. The Thread class is two different things, and they seem to have mixed the two without documentation.
    This is not common knowledge because it is not documented. They should say explicitly that all dying threads will call notifyAll on their respective objects.
    They should not have waited on the thread object itself during join, but waited on some other internal UNEXPOSED object. Like say, a JOIN OBJECT!? This is in violation of the principle of encapsulation. Internal behavior of the object is exposed on its surface. I guess they got cought and of course they can't change it now precisely because its out like a loosed pigeon.
    That is a joke.
    The simple truth is when I call wait, I am waiting on an OBJECT, when I call join I am joininig a THREAD. How could they mix these two and sleep at night?
    They shold have the common decency to explain their mistake clearly in the documentation. They did with thread.stop().
    Embarassing.
    [ March 10, 2004: Message edited by: CL Gilbert ]
    Michael Morris
    Ranch Hand

    Joined: Jan 30, 2002
    Posts: 3451
    Don't get so excited CL But you're right, it shouldn't be done that way, and I still wonder if it is not a violation of the spec, at least in spirit. I suppose, as Jim mentioned, there is nothing in either spec that says that a dying thread can't call notifyAll, but I suspect nobody thought about someone actually acquiring the monitor on a thread object when the native critical section code was being implemented. The lesson here as I see it is, never wait on any Runnable as the results may not be deterministic.
    Mr. C Lamont Gilbert
    Ranch Hand

    Joined: Oct 05, 2001
    Posts: 1170

    True. And your supposed to recheck your condition when you are notified anyway, so good code should not be hurt too bad.
    nevertheless, what if some other piece of code has the lock on your thread object. If your thread requires the lock in order to call notify, that thread can not exit. Well it can exit technically, but it can't be joined...
    David Weitzman
    Ranch Hand

    Joined: Jul 27, 2001
    Posts: 1365
    I'm not sure why this bothers anyone. It's well-established rule that wait() can return at any time -- if you expose an object publicly in any way it becomes fair game for synchronizers everywhere. This spawns the long tradition of only using wait() in a loop, and code respecting that principle will continue to work properly as it always has in the past while code that doesn't will continue to be broken as it always has been in the past.
    They should not have waited on the thread object itself during join, but waited on some other internal UNEXPOSED object.
    By that logic no class should ever synchronize on itself, since someone else might be trying to use it as a lock for something else. In fact, that logic is perfectly valid and I would probably follow it if I were writing a language. But alas, it's not the Java way (consider the synchronized method modifier and the fact that every class can be used as a lock). An object's moniter is very public, and you can't protect it. Many classes assume that their users have the goodwill not to randomly lock them from a different thread, and so as a general rule you shouldn't just lock random classes from random threads.
    I think the bottom line is that whether or not a class synchronizes on itself is an implementation detail that your code shouldn't depend on or interfere with. After all, if I feel that I can simply use some random object as a lock too and even if trust it to never notify me of anything, who's to say that some other library I'm using hasn't also taken the same object to be its own lock when I passed it to a library subroutine? I can't control that.
    So if you want a clean lock and you like encapsulation: make your own. There's a known safe way to do it, and it can't possibly cause problems of any sort.
    If you don't mind outstepping the bounds of what you can control, hoping that no other threads happen to control an object you want to get a lock for (and most of the time they don't): take the risk and see what happens.
    [ March 11, 2004: Message edited by: David Weitzman ]
    Michael Morris
    Ranch Hand

    Joined: Jan 30, 2002
    Posts: 3451
    I'm not sure why this bothers anyone. It's well-established rule that wait() can return at any time -- if you expose an object publicly in anyway it become fair game for synchronizers everywhere. This spawns the long tradition of only using wait() in a loop, and code respecting that principle will continue to work properly as it always has in the past while code that doesn't will continue to be broken as it always has been in the past.
    No Java developer worth his salt would write code like this, so that is not at issue. What bothers me is the disregard for the specification which tells me I can depend on a certain behavior yet the very people who wrote the spec don't provide that behavior in their VM.
    Mr. C Lamont Gilbert
    Ranch Hand

    Joined: Oct 05, 2001
    Posts: 1170

    Its not well established because its not documented. It is ok if thread exit calls notifyAll(), as long as we all know it does. Nor is it established that wait() can return at any time. Programs should behave in a determinate fashion.
    I agree with David's assertion about classes synchronizing on themselves. However, an object's monitor is no more public than the object itself.
    Admittly, I got a lot of code to change now I write my code largely because I enjoy using Java. Thus, I try to make it as perfect as possible.
    David Weitzman
    Ranch Hand

    Joined: Jul 27, 2001
    Posts: 1365
    Its not well established because its not documented. It is ok if thread exit calls notifyAll(), as long as we all know it does. Nor is it established that wait() can return at any time. Programs should behave in a determinate fashion.
    It's not documented in the wait() docs because that's not where the source of spurious waking is. The source of "random" waking is in the design patterns we frequently use that treat arbitrary objects as locks. Because that's common practice and unpreventable, it's vital to protect yourself against it. Using loops even where you don't need them is a good idea because then you'll never have to worry that you might have accidentally missed a case where a private object was passed out of your control.
    It's not Sun's job to recommend sound patterns of usage, but even so this might be an exception. The proposed final draft of JSR-166 (the new concurrency stuff in 1.5) was submitted three days ago, but they can still make "clarifications and corrections." Does anyone have any thoughts on what the javadocs should say about this?
    Stefan Wagner
    Ranch Hand

    Joined: Jun 02, 2003
    Posts: 1923

    Originally posted by Jim Yingst:
    [b]But I ran the code, and observed results similar to MM's. About 60% of the time, all three threads block on "waiting..." as expected. 35% of the time though, all three threads complete with "Got lock". Less than 5% of the time, one or two threads wait, while remaining threads complete. For what it's worth, I'm running J2SDK 1.4.2_03 on XP Pro.

    Test on Linux kernel 2.4.22, jdk 1.4.2-beta-b19:
    100% ended with 'Got lock'. (20 Runs)
    same result with 1.4.2_02-b3


    http://home.arcor.de/hirnstrom/bewerbung
    Warren Dew
    blacksmith
    Ranch Hand

    Joined: Mar 04, 2004
    Posts: 1332
        
        2
    Originally posted by David Weitzman:
    Does anyone have any thoughts on what the javadocs should say about this?

    In answer to your earlier question, the behavior doesn't bother me, but the fact that it's impossible to figure out just from the documentation does. The documentation should really fully document the behavior of everything in the language.
    I think the JavaDoc for the Thread class Run method should specify that it calls the notifyAll() method on completion, if that's what actually happens. If it's implementation dependent, the documentation should state that - but I think it would be bad for it to be implementation dependent, as the great strength of Java as a cross platform language lies in the ability to reproduce not only correct behavior but also as many bugs as possible across platforms.
    Warren
    Michael Morris
    Ranch Hand

    Joined: Jan 30, 2002
    Posts: 3451
    Does anyone have any thoughts on what the javadocs should say about this?
    If we take action, do we go thru the formal process of filing a bug report which some Sun engineer may subjectively dismiss out of hand or try to get clarification from some other source? I've gone thru all the texts I have on Java threads and monitors and have found nothing that warns a developer not to lock on a Runnable. That indicates to me that very few in the industry are aware of this. But as David says good practice would preclude one from having a problem with this anyway.
    David Weitzman
    Ranch Hand

    Joined: Jul 27, 2001
    Posts: 1365
    It seems there are a few options for policy-type people.
    1) Declare that whether or not a client notifies itself is the choice of the client and such choices don't need to be specifically documented. The problem with this option is that using any standard Java class as a lock become risky, so all locks would need to be private Object instances. A call to libraryObj.function() could conceivably do this:

    If this were a general policy, though, I'm not sure that the documentation could go into a specific class. Synchronization is a language feature, not a class. Certainly it wouldn't hurt to have a warning in the Object.wait() method and at the top of the Thread class linking to the statement, wherever it might be.
    2) Declare that this is a bug in the Thread implementation and modify it to use a private lock. There may be other classes in the foundation classes with similar bugs that need fixing. Class libraries not released by Sun may abide by their own policies.
    3) Document this behavior in the Thread class and any other foundation classes that synchronize on or notify themselves (as well as any objects they return).
    Realistically I think if I were Sun I'd go with option 1 but remove all instances of synchronized(this) in the codebase just for the sake of information hiding. It's good to get people in the habit of not making assumptions about undocumented behavior of other code.
    David Weitzman
    Ranch Hand

    Joined: Jul 27, 2001
    Posts: 1365
    If we take action, do we go thru the formal process of filing a bug report which some Sun engineer may subjectively dismiss out of hand or try to get clarification from some other source?
    That would be hopeless
    I was thinking more in terms of posting stuff on the JSR-166 mailing list or for those of you who use gmane it's at available at gmane.comp.java.jsr.166-concurrency.
    Mr. C Lamont Gilbert
    Ranch Hand

    Joined: Oct 05, 2001
    Posts: 1170

    Originally posted by Michael Morris:
    Does anyone have any thoughts on what the javadocs should say about this?
    If we take action, do we go thru the formal process of filing a bug report which some Sun engineer may subjectively dismiss out of hand or try to get clarification from some other source? I've gone thru all the texts I have on Java threads and monitors and have found nothing that warns a developer not to lock on a Runnable. That indicates to me that very few in the industry are aware of this. But as David says good practice would preclude one from having a problem with this anyway.

    The problem here is locking on the Thread object itself. The problem is not locking on any Runnable that may be passed into it. Synchronizing on the method level is just plain bad OO. But the specific problem here is synchronizing on the method level wrt/ the Thread class, not Runnable.
    Mr. C Lamont Gilbert
    Ranch Hand

    Joined: Oct 05, 2001
    Posts: 1170

    Originally posted by David Weitzman:
    It seems there are a few options for policy-type people.
    2) Declare that this is a bug in the Thread implementation and modify it to use a private lock. There may be other classes in the foundation classes with similar bugs that need fixing. Class libraries not released by Sun may abide by their own policies.


    But thats the problem when you violate the Encapsulation principle. You can't go back. If they make this change, any program that happens to be synchronizing on a Thread based object, or waiting on a Thread based object, will have its behavior affected.


    3) Document this behavior in the Thread class and any other foundation classes that synchronize on or notify themselves (as well as any objects they return).

    This is the only option they realistically have.

    Realistically I think if I were Sun I'd go with option 1 but remove all instances of synchronized(this) in the codebase just for the sake of information hiding. It's good to get people in the habit of not making assumptions about undocumented behavior of other code.

    As an extension of this, the synchronize keyword should be removed from methods. By definition it is a violation of encapsulation.
    [ March 16, 2004: Message edited by: CL Gilbert ]
    Michael Morris
    Ranch Hand

    Joined: Jan 30, 2002
    Posts: 3451
    I received confirmation from Sun today. They are tracking this as a VM bug. Here is the link on the Bug Parade:
    http://developer.java.sun.com/developer/bugParade/bugs/5031999.html
     
    I agree. Here's the link: http://aspose.com/file-tools
     
    subject: Unexpected thread behavior