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

Kim Barrett kim.barrett at oracle.com
Fri Aug 19 23:09:27 UTC 2016

> On Aug 13, 2016, at 4:07 PM, Mandy Chung <mandy.chung at oracle.com> wrote:
>> On Aug 13, 2016, at 1:20 AM, Peter Levart <peter.levart at gmail.com> wrote:
>> Hi Mandy,
>> On 08/13/2016 01:55 AM, Mandy Chung wrote:
>>>> On Aug 8, 2016, at 6:25 PM, Kim Barrett <kim.barrett at oracle.com>
>>>> wrote:
>>>> full: 
>>>> http://cr.openjdk.java.net/~kbarrett/8156500/jdk.04/
>>>> http://cr.openjdk.java.net/~kbarrett/8156500/hotspot.04/
>>> This looks very good.  
>>> Have you considered having JVM_WaitForReferencePendingList method to return the pending list?  i.e. make it a blocking version of getAndClearReferencePendingList rather than two operations.
>>> waitForReferenceProcessing is really wait for pending reference(s) enqueued (additionally cleaner gets invoked).  What do you think to rename this method to “waitForPendingReferencesEnqueued”?
>> I think the split is intentional. It's a clever way to avoid an otherwise inevitable race that could otherwise cause DBB allocating thread to fail with OOME prematurely.
> Ah I see.  I initially read that pendingList doesn’t need to be set within the synchronized block with processPendingActive flag but such race could cause DBB to fail with OOME, as you clarified.
> Then I suggest waitForReferencePendingList (and its JVM function) be renamed to:
>   boolean awaitReferencePendingList();
> a blocking version of "boolean hasReferencePendingList()”.  Minor difference but I think it’s little clearer.

Regarding the name, e.g. "waitFor" vs "await", the meaning is the
same.  If there were a documented or defacto preference for "await" I
would feel obliged to rename, but the JDK seems to use both forms in
roughly comparable numbers.

Regarding the signature and description, recall that
waitForReferencePendingList() does not return a value, nor does it
need to.  So documenting it as a blocking version of
hasReferencePendingList() isn't correct.

> waitForReferenceProcessing - I’d like the javadoc to include more details or @implNote for example: 
>  This method returns true to indicate that some pending references have been enqueued.   If there are cleaners, cleaning actions are queued to be invoked e.g. DirectByteBuffer may be deallocated.
> This would help the readers to understand what this method is intended to do.

I was intentionally a little vague in the description; I don't want to
be promising too much, and having callers depend on behavior that we
might want to change. In particular, it was intentional that there's
no mention of Cleaner or any special behavior for them.

For example, we might later decide we don't want the reference
processing thread to incur the cost of notifying possible waiters and
the associated synchronization, and instead let "waiters" return
immediately when there is reference processing in progress, on the
assumption that the reference processing thread makes rapid progress.
In some respects that's probably more similar in effect to what the
old code did.

Also recall that this function is presently private, accessed by
nio.Bits code via SharedSecrets (and via reflection by one test).  It
might be appropriate to change the access to this function or provide
some similar non-private function in the future, at which point the
javadoc and possibly the semantics and implementation will need to be
carefully examined.

More information about the core-libs-dev mailing list