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

[Daniel Fuchs]

On 30/07/15 12:20, Peter Levart wrote:
>
>
> On 07/30/2015 11:12 AM, Daniel Fuchs wrote:
>> Hi Peter,
>>
>> I'm glad you're looking at this too! We can't have too many eyes :-)
>>
>> On 30/07/15 10:38, Peter Levart wrote:
>>> Suppose we have a Reference 'r' and it's associated ReferenceQueue 'q'.
>>> Currently it can happen that the following evaluates to true, which is
>>> surprising:
>>>
>>> q.poll() == null && r.isEnqueued()
>>>
>>
>> But on the other hand this can only happen if two different
>> threads are polling the queue - in which case only one of them
>> will get the reference. In such a situation, the symmetric condition
>> would not be surprising (as the other thread would
>> get q.poll() != null):
>>
>>    r.isEnqueued() && q.poll() == null
>>
>> The original bug fixed by Kim is more surprising, because there's only
>> one Thread doing the polling, and it does get:
>>
>>    r.isEnqueud() && q.poll() == null
>>
>> although nobody else is polling the queue.
>> This should no longer occur in this situation with Kim's fix.
>>
>> cheers,
>>
>> -- daniel
>
> Yes, these are two different issues. The one Kim has fixed is the race
> of above expressions with Queue.enqueue() (when the queue is changing
> from the associated instance to ENQUEUED). The one I'm pointing at and
> Kim has already identified as potential issue is the race of the
> following expression:
>
> r.isEnqueued() && q.poll() == null && r.isEnqueued() ==> true
>
> ....with Queue.[realy]poll() (when the queue is changing from ENQUEUED
> to NULL).
>
> Which only happens if at least two threads are polling the queue, but is
> equally surprising and prone to cause bugs, don't you think?

Hi Peter,

Yes - this is also surprising. Is it prone to cause bugs?
possibly - but how serious I'm not sure.
Is it 'equally' surprising - well - that was the point of my argument:
there are two threads polling the same queue - so one of them should
get null... Though I agree that in this case - 'r' should be seen
has having changed state...

The question for me is whether this should be fixed in the same
changeset - or whether we should make it in another changeset...

cheers, and thanks for pointing it out!

-- daniel

>
> Regards, Peter
>
>