On Wed, 26 Aug 2026 10:29:22 GMT, Per Minborg <[email protected]> wrote:
>> 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? Relying on `isAlive()` would only work for confined arenas, right? Otherwise there is a race between different threads calling `close()`. The result of `isAlive()` before the call to `super.close()` might be 'stale' if the session gets closed in another thread right after `isAlive()` returns, but before `super.close()` is called. I'm also trying to think about this from the angle of: what is a user supposed to do when they write a custom arena type, as they don't have access to `justClose()`. You've discovered an interesting conundrum. I think one thing we could do is register the cleanup of the pool as a cleanup action. The code in the base class makes sure that all cleanup actions will run, even if one of them throws an exception. An external user would do that by wrapping the pool address in a `MemorySegment` and then calling `reinterpret` on the wrapped arena with their custom cleanup logic. But, we could just attach the cleanup action to the session directly. However, that will add some overhead because the cleanup action callback has to be created. I think what your work has shown is that users can not just run additional cleanup logic in the `close()` method. Thanks for explaining the issue. If this isn't the quick fix I thought it was I'm fine with the current version you have. Though, this is probably something we still need to come up with a definitive answer for for external users as well. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31365#discussion_r3862880022
