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