On Tue, 18 Aug 2026 18:29:24 GMT, Jorn Vernee <[email protected]> wrote:
>> The reason is that the cleanup actions could throw. I will add a comment on
>> that in the code.
>
> Could we use try/finally? From my reading of the current code, `justClose`
> and `cleanup` are not intended to be called outside of `MemorySessionImpl`,
> just overridden. I think we should try to keep it that way (and keep
> `close()` being the single source of truth for how the two interact).
>
> Suggestion:
>
> try {
> super.close();
> } finally {
> if (pool > 0) {
> ConfinedSegmentPool.release(session.owner, pool, poolSp);
> }
> }
While the effort of using only `super.close()` is good, I've failed to come up
with something similar that works. If we add a `try/finally` solution like the
one proposed, the following can happen inside `super.close()`:
1. `justClose()` fails because the arena is acquired or closed from the wrong
thread. The session remains open, but the pool is released. Live segments could
then alias a reused pool.
2. `justClose()` succeeds, but the cleanup actions throw. The session is now
closed, and the pool is released. If the session is closed again, the `finally`
block releases the same pool twice.
3. A second close of the arena fails with "already closed" but again the pool
is released twice.
There are probably more of these variants.
Capturing specific exceptions does not work, as user code can throw arbitrary
exceptions in cleanup actions.
If anyone can see a better solution, please let me know.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31365#discussion_r3861763064
