[jdk18] RFR: 8279294: NonblockingQueue::try_pop may improperly indicate queue is empty

[Kim Barrett]

On Tue, 18 Jan 2022 09:53:22 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>> Please review this improvement to NonblockingQueue::try_pop.  The old code
>> returned an indication that the queue was empty in some cases where that
>> wasn't true.  In particular, contending try_pop operations could result in
>> some incorrectly indicating empty.  The change fixes that and improves the
>> interaction between contending try_pops.
>> 
>> Testing:
>> mach5 tier1
>> 
>> Lots of testing of this change in conjunction with others as part of
>> investigating and fixing JDK-8273383.
>
> src/hotspot/share/utilities/nonblockingQueue.inline.hpp line 144:
> 
>> 142:     // (1) next_node is the extension of the queue's list.
>> 143:     // (2) next_node is NULL, because a competing try_pop took result.
>> 144:     // (3) next_node is the extension of some unrelated list, because a
> 
> Would it be possible to name these cases a/b/c instead of numbering them? The comments below also refer to these subcases as 1a-1c.
> 
> Cases a) and c) in this list seem to be in a different order than in the code, would be nice if they matched (just exchange).
> 
> I do not understand (a linguistic problem) why for case 3, `next_node` is the extension of some "unrelated" list, i.e. I do not understand the use of the adjective "unrelated" here, probably taking the word too literally.
> 
> The `result` to be popped must be in the same list as the list other threads pop from (e.g. in this case we always pop from the `_head` of the completed buffers list, racing with threads popping from the `_head` of the same list. There does not seem to be a way to compete with other, different lists here). Even if we got some element of a sub-list of the original list, it and its sublists seem still "related".

The labeling is intentionally different.  The two groups are not congruent.  I thought
there was enough commentary there to make things clear, but apparently not.

-------------

PR: https://git.openjdk.java.net/jdk18/pull/106