Code Review request for bug fixes found by the Contended Locking project

[Daniel D. Daugherty]

On 1/20/13 4:08 PM, David Holmes wrote:
> Hi Dan,
>
> Thanks for all your efforts on this. Reviewed.

Thanks!


> Minor nit: There seem to be some pre-existing indentation issues in a 
> couple of places that you've got caught in:
>
> eg src/os/bsd/vm/os_bsd.cpp (and linux & solaris)
>
>     _Event = 0 ;
>      status = pthread_mutex_unlock(_mutex);
>      assert_status(status == 0, status, "mutex_unlock");
>     // Paranoia to ensure our locked and lock-free paths interact
>     // correctly with each other.
>     OrderAccess::fence();

My general rule of thumb: if I touched the line for another reason,
then I fixed formatting issues (indents, spaces before '(', space
before ';', etc.). For example, in os::PlatformEvent::unpark(),
the "if (v < 0) {" was removed which caused me to shift an entire
block of code to the left. I considered that a valid reason to fix
all the formatting issues in the block.

In the above example, I didn't have a reason to touch the lines
above the ones that I added so I didn't fix them, yet.


> Actually there seems to be a lot of incorrect indenting in the 
> os_<os>.cpp files :(

Yes there is. I have much of that fixed in my "cleanup" bucket for the
Contended Locking project.

Again, thanks for the review.

Dan

>
> Thanks again,
> David
>
> On 18/01/2013 4:45 AM, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> I have been working on the Contended Locking project and there are some
>> bug fixes that I'd like to push into HSX-25 independently of the 
>> Contended
>> Locking project.
>>
>> I'm using three different bug IDs to track these very distinct bug
>> fixes:
>>
>> 6444286 Possible naked oop related to biased locking revocation
>> safepoint in jni_exit()
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6444286
>> https://jbs.oracle.com/bugs/browse/JDK-6444286
>>
>> 8004902 correctness fixes motivated by contended locking work (6607129)
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8004902
>> https://jbs.oracle.com/bugs/browse/JDK-8004902
>>
>> 8004903 VMThread::execute() calls 
>> Thread::check_for_valid_safepoint_state()
>> on concurrent VM ops
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8004903
>> https://jbs.oracle.com/bugs/browse/JDK-8004903
>>
>> Here is the URL for the webrev:
>>
>> http://cr.openjdk.java.net/~dcubed/cl_bug_fix_bucket-webrev/2/
>>
>> These changes have been through two internal rounds of code review so I
>> think they are ready for wider review. The changes for 8004902 are very
>> tricky and there is long write-up attached to the 8004902 bug report 
>> that
>> explains the intricate details of the "triangular race". The changes for
>> 6444286 and 8004903 are more straight forward.
>>
>> These changes have been through JPRT build-and-test and have also been
>> tested with the vm.parallel_class_loading and vm.quick subsuites via
>> the Aurora ad-hoc testing mechanism.
>>
>> The current set of reviewers is:
>>
>> acorn, dholmes, dice
>>
>> As always, other reviewers are welcome. Comments, questions and
>> suggestions are also welcome.
>>
>> Dan
>>