Andrew John Hughes wrote:
2009/9/21 Daniel D. Daugherty <[email protected]>:
Second Call for any OpenJDK reviewers!

I will be closing the review window Tuesday (09.22) evening on this fix.

Dan


Daniel D. Daugherty wrote:
Greetings,

I'm looking for a couple of OpenJDK code reviewers for my fix to
the following bug:

  6876794 4/4 sp07t002 hangs very intermittently

This one is a classic suspend/resume three-way deadlock caused by
a case of over locking so I figured I would give this one a wider
review.

Here is the URL for code review round 0 webrev:

  http://cr.openjdk.java.net/~dcubed/6876794-webrev/0/

I'm targeting this fix for HSX-17-B02 so I'd like to hear back
from at least one reviewer in a week or so...

Thanks, in advance, for any feedback!

Dan



Hi Dan,

First of all, thanks for posting this for public review.

You're welcome.


It's a shame
more people haven't responded, but then I guess fairly intimate
HotSpot code like this may scare most people off :)

Hopefully, these reviews and the verbose comments will help with
that situation.


Assuming your analysis of the locking is correct (it sounds that way
to me), then this seems like a fairly straightforward patch.

So far no one has found any holes in the logic :-) But then again it
took a few years to find this one :-)


From
what I can see, most of it is cleanup; the
is_any_suspended_with_lock() method that is removed is presumably not
called from anywhere (I don't see any call removal in the patch)

The one remaining use of is_any_suspended_with_lock() was line 774
in safepoint.cpp and it was replaced with is_ext_suspended() on
new line 788 in safepoint.cpp.


and
replacing is_any_suspended with its single line contents is also
straightforward.

And that was the only use of is_any_suspended().


So I also agree with the patch.

Thanks for the review. I'll add you to the reviewers list.

Dan

Reply via email to