RFR: 8351374: Improve comment about queue.remove timeout in CleanerImpl.run

[Brent Christian]

On Wed, 12 Mar 2025 19:57:23 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/ref/CleanerImpl.java line 142:
>> 
>>> 140:                 // while there are registered cleanables for other objects.
>>> 141:                 // If the application cleans all remaining cleanables, there
>>> 142:                 // won't be any references enqueued to unblock this.  Using a
>> 
>> I agree that calling `queue.remove()` with a timeout is the right approach.
>> But I have a question:
>> In the case where the Cleaner's `CleanerCleanable` has already run, and we get to processing the last registered cleanable on `activeList`:
>> When we do the `ref.clean()`, the `activeList` becomes empty, and we'll drop out of the `while()` loop. So I'm not seeing how we would attempt another `queue.remove()` in this case.
>> What am I missing?
>
> You are missing that this loop is not the only place that potentially removes
> references from `activeList`. The application may also do so, when directly
> cleaning (as part of a `close()` operation or the like). So the cleaner's
> cleanable may be dropped, removed, and cleaned but with live entries still in
> `activeList`, and this loop ends up blocked in `remove` because there's
> nothing for it to do.  The application later closes all of the remaining
> entries in the `activeList`, which doesn't cause anything to be enqueued on
> the cleaner's queue, so the cleaner thread remains blocked in `remove`.  But
> now `activeList` is empty and will never be added to, and the queue is also
> empty, and the thread is blocked in `remove` with nothing (other than the
> timeout) to break it out.

Got it, thanks.

I'd like to suggest a small update:
_If the application explicitly calls clean() on all remaining Cleanables, there..._

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23952#discussion_r1994219044