On Mon, 10 Mar 2025 23:14:23 GMT, Brent Christian <bchri...@openjdk.org> wrote:
>> Please review this revision of a previously puzzling comment intending to >> provide the rationale for a bit of non-obvious code. > > 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23952#discussion_r1992205243
