RFR (XS): 8014890 : Reference queues may return more entries than expected

[Thomas Schatzl]

Hi,

 one more note :)

On Wed, 2013-06-19 at 09:28 +0200, Thomas Schatzl wrote:
> Hi again,
> 
>   some correction...
> 
> On Wed, 2013-06-19 at 09:17 +0200, Thomas Schatzl wrote:
> > Hi,
> > 
> > On Wed, 2013-06-19 at 12:47 +1000, David Holmes wrote:
> > > Hi Thomas,
> > > 
> > > On 19/06/2013 7:08 AM, Thomas Schatzl wrote:
> > > > Hi all,
> > > >
> > > >    can I have reviews for the following change?
> > > >
> > > > It happens if multiple threads are enqueuing and dequeuing reference
> > > > objects into a reference queue, that Reference objects may be enqueued
> > > > at multiple times.
> > > >
> > > > This is because when java.lang.ref.ReferenceQueue.poll() returns and
> > > > inactivates a Reference object, another thread may just be during
> > > > enqueuing it again.
> > > 
> > > Are we talking about different queues here? Otherwise both poll() and 
> > > enqueue() are using synchronized(lock). I also note that enqueue 
> > > synchronizes on the Reference itself, which suggests that poll (actually 
> > > reallyPoll) should also be synchronizing on the reference (though we 
> > > have a nested lock inversion problem that would need to be solved).
> > 
> > This does not help imo. Still there may be temporary storage containing the original ReferenceQueue reference, and .enqueue() gets called on it with the now inactive Reference.
> > 
> > Enqueue() and (really-)poll() themselves already synchronize on the
> > "lock" lock to guard modification of the queue, this is safe.
> > 
> > The synchronize(r) statement in ReferenceQueue.enqueue() is about synchronization with Reference.isEnqueued() I think. Actually I am not sure whether the synchronization between isEnqueued() and enqueue() makes a difference.
> 
> This also guards against multiple concurrent calls to enqueue on the
> same reference - so this statement is needed after all.

Actually, with that patch (e.g. the if (this != r.queue check) return
false; ) this situation would also be covered I think.

But that's unrelated, sorry.

Thomas