On Thu, 23 Feb 2023 15:13:23 GMT, Daniel Jeliński <[email protected]> wrote:
>> Please review this fix for a race in KeepAliveCache. >> >> The sweeper thread (`KeepAliveCache.run()`) terminates when the cache is >> empty. The check for empty cache was performed without holding the cache >> lock. As a result, there was a small window of time where a new connection >> could be added to the cache while the sweeper thread was stopping. The added >> connection would then be removed from cache without closing when a new >> sweeper thread was started. >> >> As an additional observation, the check for empty cache was performed after >> sweeping. The cache could be empty because all connections were closed, or >> because all connections were busy. In the latter case, a new thread was >> created soon after the old one terminated. >> >> This patch addresses both these issues. The sweeper thread makes the >> decision to terminate while holding the lock, and other threads are aware of >> that when they acquire the lock. Also, the sweeper thread only terminates if >> the cache is empty AND there was no cache activity in the last `LIFETIME` (5 >> seconds). >> >> Tier1-3 clean. No new tests, the issue was very hard to reproduce without >> adding delays in production code.. > > Daniel Jeliński has updated the pull request incrementally with one > additional commit since the last revision: > > Add asserts Marked as reviewed by dfuchs (Reviewer). ------------- PR: https://git.openjdk.org/jdk/pull/12676
