JDK-8052260 (Reference.isEnqueued()) Discussion

[Hans Boehm]

Thanks!

I thought the current spec fairly clearly states something other than what
the implementation does:

"Tells whether or not this reference object has been enqueued ..."

I do agree that the name is inconsistent with that.  If I didn't know
better, I would still believe the sentence in the spec, since it  gives you
a stable "true" result, as opposed to an answer that could always be
falsified by the time the method returns.

I agree that code review SHOULD catch such bugs.  It's possible that it
caught most of them and 99.9% of code that used to use isEnqueued() no
longer uses isEnqueued() as a result.  But I'm not that much of an optimist.

On Thu, Feb 4, 2016 at 4:52 PM, Mandy Chung <mandy.chung at oracle.com> wrote:

>
> > On Feb 4, 2016, at 2:37 PM, Hans Boehm <hboehm at google.com> wrote:
> >
> > The discussion at https://bugs.openjdk.java.net/browse/JDK-8052260
> > correctly points out that the spec implies that this should be true if
> the
> > reference has ever been enqueued, while the implementation returns false
> > once it has been removed from the queue.  The current proposal is to
> change
> > the spec.
>
> The spec needs clarification (not a spec change and no behavioral change).
>
> >  I'd like to make a different proposal, based on the following
> > observations:
> >
> > 1) I don't know of any way to write correct code that is oblivious to
> this
> > difference.
> > 2) AFAIK, this has been broken in this way for a long time, possibly from
> > the beginning, i.e. for more than 15 years(?).  The bug is dated 2014.
> > Apparently nobody noticed for a long time.
>
> AFAIK nothing is broken.
>
> > 3) I don't know of any useful correct code that uses the function as
> > implemented.  The implemented and proposed semantics seem to be
> inherently
> > racey, at least if the associated queue is processed asynchronously.
> > Martin Buchholz suggested debugging statistics as a possible use.  That
> > seems to be the best answer to date, but it still seems to me that a heap
> > profile would generally serve better in such cases.
> > 4) I've looked at as many clients as I could find.  The number is in the
> > single digits. I don't believe any of them are correct with the current
> > implementation.  The majority misuses isEnqueued() on References without
> an
> > associated queue as a test to see if the reference has been cleared.
>
> Ah, I see. Hmm.. should code review catch this?
>
> The spec is clear on this point though:
>
> If this reference object was not registered with a queue when it was
> created, then this method will always return false.
>
> > In one case this introduced a potentially serious bug; in other cases it
> just
> > slowed down the code.
> >
> > My conclusion is that this call currently only serves to inject bugs and
> > slowdowns into code written by unwary users.  I suggest deprecating it.
>
> Fair point while deprecation may not be the right answer to address
> “mis-reading” of the spec.   On the other hand, this method does not seem
> to be useful.  I have included your comment in the JBS issue to consider.
>
> Mandy