United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6562569 REGRESSION: can not see BLOCKED state change after patching 5104215 diff.
JDK-6562569 : REGRESSION: can not see BLOCKED state change after patching 5104215 diff.

Details
Type:
Bug
Submit Date:
2007-05-28
Status:
Closed
Updated Date:
2011-09-22
Project Name:
JDK
Resolved Date:
2011-03-08
Component:
hotspot
OS:
windows_xp
Sub-Component:
jvmti
CPU:
x86
Priority:
P4
Resolution:
Fixed
Affected Versions:
6u2
Fixed Versions:
hs11 (b08)

Related Reports
Backport:
Backport:
Backport:
Relates:
Relates:

Sub Tasks

Description
After patching 5104215 fix(that is a fix in jdk5.0u6), the attached 
test program can not watch that the thread becomes BLOCKED state.

Below is the snippet related to Thread.State from JDK 5.0 API's doc.
http://java.sun.com/j2se/1.5.0/docs/api/java/lang/Thread.State.html
-----
BLOCKED
Thread state for a thread blocked waiting for a monitor lock. A thread
in the blocked state is waiting for a monitor lock to enter a
synchronized block/method or reenter a synchronized block/method after
calling Object.wait.

WAITING
Thread state for a waiting thread. A thread is in the waiting state
due to calling one of the following methods:
Object.wait with no timeout 
Thread.join with no timeout 
LockSupport.park 
A thread in the waiting state is waiting for another thread to perform
a particular action. For example, a thread that has called
Object.wait() on an object is waiting for another thread to call
Object.notify() or Object.notifyAll() on that object. A thread that
has called Thread.join() is waiting for a specified thread to
terminate.  
-----

Test Program:
-----
public class MustBeBlocked extends Thread {
  static Object _lock = new Object();

  public static void main(String[] args) throws Exception {
    Thread t = new MustBeBlocked();
    t.start();
    
    do {
      Thread.sleep(100);
    }while(t.getState()!=Thread.State.WAITING);

    synchronized(_lock) {
      _lock.notify();
      while( t.getState()==Thread.State.WAITING) {  // Must be BLOCKED
        System.out.println("waiting .."); 
        Thread.sleep(1);
      }
      System.out.println( t.getState()); 
    }
    System.exit(0);
  }

  public void run() {
    try { run0();}
    catch(Exception e){e.printStackTrace();}
  }
  
  public void run0() throws Exception{
    synchronized(_lock) {
      _lock.wait();
    } 
  }
}
-----
The TP's purpose is based on API's BLOCKED part "or reenter a
synchronized block/method after calling Object.wait.".
main() watches if the thread will become BLOCKED state after it is
notified from wait(). When the state becomes BLOCKED, the program 
will exit.

Expected result:
(1) JDK5.0u5
# java MustBeBlocked
waiting ..
BLOCKED

Current result:
(2) JDK5.0u12(b04)
# java MustBeBlocked
waiting ..
waiting ..
waiting ..
waiting ..
waiting ..
waiting ..
waiting ..
waiting ..
...

(3) JDK6u2(b02)
# java MustBeBlocked
waiting ..
waiting ..
waiting ..
waiting ..
waiting ..
waiting ..
waiting ..
waiting ..
...

                                    

Comments
EVALUATION

I can't see how the fix of the 5104215 causes this regression.
The only fix for the 5104215 was this:

% sccs prt threadService.hpp
D 1.29	05/07/18 17:25:00 mchung	39 38	00001/00001/00472
5104215: Blocked thread status is incorrectly reported as runnable

ss45998@tomsk sccs sccsdiff -r1.28 -r1.29 threadService.hpp
------- threadService.hpp -------
448c448
<     if (is_alive() && ServiceUtil::visible_oop((oop)obj_m->object()) && obj_m->owner() !=NULL) {
---
>     if (is_alive() && ServiceUtil::visible_oop((oop)obj_m->object()) && obj_m->contentions() > 0) {

The fix above is just to better identify a condition when the thread can
be moved to the BLOCKED_ON_MONITOR_ENTER state.
So, I think, the 5104215 fix was not able to cause this regression.
Most likely, the regression came from another fix.

It seems to me that the real source of the bug is located in the code of
the functions: ObjectMonitor::notify and ObjectMonitor::notifyAll.
I see that Dave has correctly marked both functions with the comment:
// TODO-FIXME: 6307490 - change thread state from WAITING to BLOCKED.

The problem is that the thread(s) being notified is/are just moved from
the _WaitSet linked list to the _EntryList or _cxq linked list.
The thread(s) is not uparked and its thread state is not updated.

Most likely (I'm not sure yet), the most accurate fix would be to unpark the
notifyee threads, otherwise potential monitor contended entry events
can't be reported (but probably, they are missed now anyway).

With the following prototype/fix the test (from description) is passed:

synchronizer.cpp:
215a216
> static int Knob_UnparkNotifyee     = 4 ;       // notify() - unpark of notifyee
4079c4080
<   int Policy = Knob_MoveNotifyee ; 
---
>   int Policy = Knob_UnparkNotifyee ; 
4198c4199
<   int Policy = Knob_MoveNotifyee ; 
---
>   int Policy = Knob_UnparkNotifyee ;

Also, I sufacely took a look into current jdk5 implementation.
The problem is still the same while the implementation of monitors
was rewritten in jdk6/jdk7 significantly.
                                     
2007-08-10
EVALUATION

This is a helpful email explanations from David Holmes below.

--------------------------------------------------------------------------------
Serguei,

You do not want to unpark the notifyees - that basically undoes the change that added the wait-morphing code (and wait-morphing is a good thing!).

By definition when a thread does a notify() and there is a waiter then the monitor becomes contended. If we need to post events for that (and aren't) then it can be added higher up the notify() chain I would think.

The issue is updating the higher-level thread-state in the Java thread. Injecting this state change from the lowest-level of the synchronization code is problematic. I would prefer to see an architecture where the thread-state is determined by probing values like the "waiting monitor" etc rather than have an actual state field that all the low-level code has to try and update at the right times. Such a change would be fairly significant in itself of course.

The simplest fix would probably be to directly set a new value into the state field, rather than trying to use the existing helper objects that "push" and "pop" the state changes - but you'd have to watch in case the higher-level helpers get surprised by the change of state.

Again, it was the backport of the wait-morphing code to 5.0u6 that caused this "regression". From 5.0u6 though 6 and 7 the behaviour is now consistent (but wrong :( ) and so one fix should be driven through for all platforms.

Cheers,
David Holmes
----------------------------------------------------------------------------------
                                     
2007-08-10
SUGGESTED FIX

The suggestion is that the "notifying" thread has to change thread state of "wating" thread.
Please, see the webrev hs_wait.Sep29 with the suggested fix in attachments.
                                     
2007-09-30



Hardware and Software, Engineered to Work Together