On 07/30/2015 12:24 PM, David Holmes wrote:
On 30/07/2015 8:20 PM, 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?
Sorry I'm missing your point. The expression:
r.isEnqueued() && q.poll() == null
is exactly the race the current fix is addressing. Adding a second
check of r.isEnqueued() which still returns true does not add anything
that I can see. ??
David
The expressions are similar, but the race is different. The one
addressed by Kim is the race of:
r.isEnqueued() && q.poll() == null ==> true
with q.enqueue(r)
There, the reversal of assignments to q.head and r.queue in
ReferenceQueue.enqueue() fixes the issue.
The one I'm pointing at is the race of:
r.isEnqueued() && q.poll() == null && r.isEnqueued() ==> true
with q.poll()
Here, the fix would be to also revert the assignments to q.head and
r.queue in ReferenceQueue.reallyPoll() this time.
The 1st race is the race with enqueue-ing and the 2nd race is the race
with de-queueing. Initially, my "surprising" expression was:
q.poll() == null && r.isEnqueued() ==> true
...but this is not representative, as it is also an expected outcome
when racing with enqueue-ing. So I added a pre-condition to express the
fact that it happens when racing with de-queueing only:
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:
if (r.isEnqueued()) {
Reference s = q.poll();
if (s == null) {
// somebody else already dequeued 'r'
assert !r.isEnqueued();
}
}
Regards, Peter
Regards, Peter
