JDK-8052260 (Reference.isEnqueued()) Discussion
hboehm at google.com
Fri Feb 5 01:17:52 UTC 2016
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
> > reference has ever been enqueued, while the implementation returns false
> > once it has been removed from the queue. The current proposal is to
> > 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
> > 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
> > 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
> > 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
> > 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.
More information about the core-libs-dev