ChrisHegarty commented on PR #16931:
URL: https://github.com/apache/lucene/pull/16931#issuecomment-6473444351
Thanks Uwe, and thanks Gianluca for the independent report. Here's the
walk-through.
The arenas map holds the current arena per group, so that new files can join
it. Each arena removed itself with arenas.remove(key), which removes whatever
is mapped to that key.
1. Open f1..f64: _1 -> A, and A is now out of permits.
2. Open f65: _1 -> B. A stays alive because 64 inputs still hold it.
3. Close f1..f64: A's last release runs arenas.remove("_1"). That removes B,
which is still live.
4. Open f66: B isn't in the map, so a third arena C is created instead of
joining B.
The fix makes the arena pass itself to onClose, and the map removal becomes
arenas.remove(key, arena). ConcurrentHashMap only removes the entry if the
stored value equals the given arena. RefCountedSharedArena doesn't override
equals, so that is an identity check, and A can no longer remove B. That is why
I added the comment about not overriding it.
It also covers the retry you mentioned. If arena.close() throws
IllegalStateException, release() can run onClose again, and a retry could
previously evict a newer arena too. Now the retry only removes an entry that is
still the retrying arena.
Memory is unmapped the same way as before: release() still calls
arena.close() when the last input closes, whether or not the arena is in the
map. So this affects only how well files share arenas, not leaks or
correctness. t`estArenasOldArenaCloseKeepsNewerArena` reproduces the scenario
above and fails without the fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
