RFR: 8156500: deadlock provoked by new stress test com/sun/jdi/OomDebugTest.java

Kim Barrett kim.barrett at oracle.com
Tue Jun 28 01:21:12 UTC 2016


> On Jun 27, 2016, at 9:01 AM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
> 
> 
> http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.00/src/share/vm/memory/universe.cpp.udiff.html
> 
> Do you need load_aquires and store_releases on set_reference_pending_list and has_reference_pending_list ?
> 
> Or rather, why do you need an atomic operation in swap_pending_list if the Heap_lock is held?
> 
> Aha, you answered the question in the header file.

Yes, you found it.

> http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.00/src/share/vm/prims/jvm.cpp.udiff.html
> 
> Can you change the boolean "wait" to "should_wait” ?

Sure.   Will be in the next webrev, if one is needed.

> http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.00/src/share/vm/runtime/thread.cpp.udiff.html
> 
> This change in initialization order makes sense to me.  We really need to preallocate OOM before initializing the system libraries. Since Throwable() doesn't check a property now, this is safe.  I don't think this wasn't safe a few months ago.  Throwable constructor should do as little as possible.

I want to follow up on David’s question and try to figure out why the old order worked before
we ensured InterruptedException was initialized by Reference.

> Is the test that you're going to add from the other bug fix? ie. OomDebugTest.java (can that have a @bug 8156500 entry in it?

OomDebugTest.java was borrowed from the proposed changes for
JDK-8153711. I'm not including it with this change, because it
requires something like the other changes for that bug, but when I ran
tests with those changes I saw failures in other tests that were
related to those changes. So that test is not yet ripe for pushing.

> So many special cases removed!  This is so nice.
> 
> http://cr.openjdk.java.net/~kbarrett/8156500/jdk.00/src/java.base/share/classes/java/lang/ref/Reference.java.udiff.html
> 
> Can you add to the popReferencePendingLIst comment that the pending list is managed by the GC, ie. gc enqueues and dequeues references to process?  Or something like that?

Sure.  Added

  The GC adds references to the list, and notifies waiting threads when
  references are added.

Will be in the next webrev, if one is needed.

> 
> This is a really good change.

Thanks for reviewing.



More information about the hotspot-dev mailing list