On Wed, 26 Aug 2026 10:17:49 GMT, Per Minborg <[email protected]> wrote:
>> 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.
Come to think about it, it is probably possible to inspect the `isAlive()`
state before and after a `super.close()` operation, and if there was a change,
it is safe to release the pool. However, this would require carefully managing
potential exception states, which would add to bytecode size and consequently
reduce C2's ability to inline. Such bytecode expansion can be mitigated by
breaking out exceptional code, but at the end of the day, it would be a
trade-off between code complexity/performance loss and strict use of only
`super.close()`. It is not easy to judge which is better.
Thoughts?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31365#discussion_r3861859045
