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

[Thomas Schatzl]

On Mon, 17 Jan 2022 08:23:37 GMT, Kim Barrett <kbarrett 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".

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

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