RFR: 8132306: java/lang/ref/ReferenceEnqueue.java fails with "RuntimeException: Error: poll() returned null; expected ref object"

[David Holmes]

On 30/07/2015 9:57 PM, Peter Levart wrote:
>
>
> On 07/30/2015 01:44 PM, David Holmes wrote:
>>> r.isEnqueued() && q.poll() == null && r.isEnqueued() ==> true
>>>
>>>
>>> What is surprising in the above expression evaluating to true is the
>>> fact that 'r' appears to be enqueued before and after the q.poll()
>>> returns null. I can easily imagine code that would fail because it never
>>> imagined above expression to evaluate to true. For example:
>>
>> So r has been enqueued and one poll() removes it, so the second poll()
>> returns NULL, but r still claims to be enqueued. Sorry I'm not seeing
>> how that is possible.
>>
>> David
>
> 'r' has been enqueued.
>
> Thread-1:
>
> r.isEnqueued() &&
> q.poll() == null &&
> r.isEnqueued()
>
> Thread-2:
>
> q.poll();
>
>
> Sequence of actions:
>
> T1: r.isEnqueued() ==> true
>
> T2: q.poll() executed to the following point (see HERE) and 'r' was the
> last element in the queue ('head' has been assigned to null):

Yeah thanks - just realized it is that darned unsynchronized "fast-path" 
again. What a mess.

It a kind of inverse of the original problem.

Original: don't update reference state to enqueued before the queue is 
updated
This one: don't update the queue state to empty before the reference 
state shows it is de-queued.

So yes the fix here is to move "r.queue = null" to before the assignment 
to head.

Bring on the next race ;-)

Thanks,
David

>      public Reference<? extends T> poll() {
>          if (head == null)
>              return null;
>          synchronized (lock) {
>              return reallyPoll();
>          }
>      }
>
>      private Reference<? extends T> reallyPoll() {       /* Must hold
> lock */
>          Reference<? extends T> r = head;
>          if (r != null) {
>              head = (r.next == r) ?
>                  null :
>                  r.next; // Unchecked due to the next field having a raw
> type in Reference
>
>              // >> HERE <<<
>
>              r.queue = NULL;
>              r.next = r;
>              queueLength--;
>              if (r instanceof FinalReference) {
>                  sun.misc.VM.addFinalRefCount(-1);
>              }
>              return r;
>          }
>          return null;
>      }
>
> T1: q.poll() finds head == null and returns null;
>
> T1: r.isEnqueued() ==> true since r.queue is still ENQUEUED
>
>
> Regards, Peter
>