Data race on ctx->threadLimit between POOL_thread() and POOL_create_advanced() #4547

Activity

1. sebres commented on Mar 1, 2026

   

   sebres

   on Mar 1, 2026

   Contributor

   More actions

   There is basically no RC at the moment, since the ctx (created here) isn't used somewhere else yet, excepting the workers in pool, where the first of them would hold the lock in:
   https://github.com/facebook/zstd/blob/ae9f20ca2716f2605822ca375995b7d876389b64/lib/common/pool.c#L72
   and then enter the conditional wait, because of ctx->queueEmpty, no matter what ctx->threadLimit is, because by || (or) the value of (ctx->numThreadsBusy >= ctx->threadLimit) is irrelevant:
   https://github.com/facebook/zstd/blob/ae9f20ca2716f2605822ca375995b7d876389b64/lib/common/pool.c#L74-L75

   But to avoid possible RC in the future (if the logic would be not enough anymore) one could indeed rewrite it a bit.
   The naive attempt to "fix" it with lock around set of #4558 is in my opinion not correct (even wise versa, can cause a sporadic deadlock).
   I'll provide a new PR for that.

   👍React with 👍2Reacted by Nick Terrell and Yuqi Guo
2. sebres

   added a commit that references this issue on Mar 1, 2026

   avoid potential RC on ctx->threadLimit, code review;

   ...

   This issue will close once commit 3587c93 is merged into the 'dev' branch.

   3587c93
3. terrelln

   added a commit that references this issue on Mar 2, 2026

   avoid potential RC on ctx->threadLimit, code review;

   ...

   This issue will close once commit 6e1e545 is merged into the 'dev' branch.

   6e1e545
4. terrelln

   closed this as completedin #4601on Mar 2, 2026