phongn opened a new pull request, #13382: URL: https://github.com/apache/trafficserver/pull/13382
## Summary `RamCacheCLFUS::compress_entries()` drops the stripe mutex around the compression call and retakes it afterward. Three bugs sat on that lock seam: 1. **Mutex imbalance.** The fastlz minimum-length check ran in the *unlocked* region and jumped to `Lfailed`, which writes to the entry, advances the compression cursor, and continues the loop — which then releases a stripe mutex this thread no longer holds (`ink_assert` in debug builds; silent lock-state corruption in release). Reachable with `compress = 1` and any resident entry under 16 bytes. 2. **Write after free.** The compression-failure check ran *before* revalidating that the entry survived the unlocked window, so a failed compression could mark `incompressible` on an entry that a concurrent `_destroy` had already freed and possibly recycled. 3. **Null cursor dereference.** When revalidation fails and the cursor was concurrently invalidated (e.g. by put-side victimization), `e = this->_compressed` can be null and the loop tail dereferences it. ## Changes - The fastlz minimum-length decision moves into the locked bound phase (same outcome: the entry is marked incompressible and skipped). - The unlocked codec switch's `default` sets the failure flag instead of jumping (unreachable today since the bound switch filters unknown types, but no unlocked jump remains). - Failure marking runs only after the entry has been revalidated under the lock. - A null cursor after failed revalidation ends the pass instead of dereferencing. After this patch the invariant is simple to audit: every write to the entry and every `Lfailed`/`Lcontinue` jump happens with the stripe mutex held; the unlocked region contains only the pure compression call on snapshotted data. ## Notes for reviewers - Observable behavior for live entries is unchanged: undersized or uncompressible entries are marked incompressible and the cursor advances, exactly as before. - Found by inspection while preparing the shared RAM-cache compression refactor. The races are timing-dependent and master has no harness that can drive `compress_entries()` synchronously (the class is private to the `.cc`), so no deterministic regression test is possible here; the compression test suite arriving with the refactor PRs includes an undersized-payload case that pins the min-len path. - Backport candidate: the same code ships in all release branches. -- 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]
