United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
JDK-8004902 : correctness fixes motivated by contended locking work (6607129)

Details
Type:
Bug
Submit Date:
2012-12-12
Status:
Closed
Updated Date:
2014-06-26
Project Name:
JDK
Resolved Date:
2013-01-22
Component:
hotspot
OS:
generic
Sub-Component:
runtime
CPU:
generic
Priority:
P4
Resolution:
Fixed
Affected Versions:
hs25
Fixed Versions:
hs25 (b17)

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

Sub Tasks

Description
Work on contended locking via 6607129 has uncovered several correctness fixes:

1a) Add missing fence() to end of os::PlatformEvent::park() on both
    Linux and Solaris
1b) Add missing barrier to Parker::park() by using Atomic::xchg()
    to query _counter field (barrier is now on both code paths)
    on Linux only.
1c) Remove assert in java_lang_Thread::set_thread_status()
1d) Add missing fence() in ObjectMonitor::EnterI() (all platforms)
1e) Remove set of "_count = 0" in ObjectMonitor::set_owner() to
    avoid race with setting of _count during inflation.

Update: forgot to include the notes section:

Notes:
1c) The java_lang_Thread::set_thread_status() change is needed for
    the pending quick monitor operations. This means the change is
    potentially needed for several other buckets so I put it in the
    bug_fix bucket as a "common" place.

                                    

Comments
1a) Add missing fence() to end of os::PlatformEvent::park() on both  Linux and Solaris 

Why is a fence needed? The code is using pthread_mutex locking which provides its own memory ordering guarantees.

1b) Add missing barrier to Parker::park() by using Atomic::xchg()  to query _counter field (barrier is now on both code paths)
    on Linux only. 

Again unclear where/why this is needed.

1c) Remove assert in java_lang_Thread::set_thread_status() 

Not that the assert seems critical but when do we set the thread status while not running _thread_in_vm?



                                     
2012-12-12
1a and 1b - Will get discussed during the code review cycle. I'm sure Dave D. had his reasons for making these two changes.

1c - I forgot to include the notes section for why 1c was in this batch.

                                     
2012-12-14
During the internal code review for this fix, I wrote a detailed analysis of the original triangular race that was fixed by 6822370
and the reasons for the other fixes associated with this bug.
                                     
2013-01-11
See attached cl_bug_fix_bucket-webrev-cr0.tgz, cl_bug_fix_bucket-webrev-cr1.tgz and cl_bug_fix_bucket-webrev-cr2.tgz for the webrevs from Code Review Round [012].

                                     
2013-01-17
URL:   http://hg.openjdk.java.net/hsx/hotspot-rt/hotspot/rev/22ba8c8ce6a6
User:  dcubed
Date:  2013-01-22 18:35:08 +0000

                                     
2013-01-22
URL:   http://hg.openjdk.java.net/hsx/hsx25/hotspot/rev/22ba8c8ce6a6
User:  amurillo
Date:  2013-01-25 20:42:57 +0000

                                     
2013-01-25



Hardware and Software, Engineered to Work Together