RFR (XS): 8014890 : Reference queues may return more entries than expected
Thomas Schatzl
thomas.schatzl at oracle.com
Wed Jun 19 07:34:05 UTC 2013
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
More information about the core-libs-dev
mailing list