Skip to content

Callback capture cleanup can deadlock last-owner destruction #112

Description

@derekste

Current fix status — 2 October 2026

The capture-cleanup defect is fixed in #108 at b6c442a04746d420742ca0640111a65286fd22e9. Completed/canceled callback captures are released outside worker locks. The normal/throwing capture-cleanup regression covers one/four workers, alongside lifecycle teardown cases. Retained Linux Release and macOS sanitizer validation pass; the exact-head build check passed. All current review threads are resolved and independent Instrumentation approval is recorded.

Keep this issue open until foundation #127 and #108 are merged and the regressions pass on merged source. IOC #100 is refreshed to this explicit development pin while remaining draft.


Historical reproduction — 30 September 2026

The callback-owned lifetime path in #108 can deadlock when the completed callback's captures release the last adapter/pool owner. This blocks merging #108 and its dependent stack until corrected. Related release tracker: fermi-ad/redis-pvxs-ioc#99.

Affected revisions: #108 at 0bb9c35e7480e5d01afd58a5ed5ce27f3191f226 and descendants, including #111 at 0c3b0e6b7fa11910afba961d065f17ac749fd345.

In ThreadPool.hpp:93, work() reacquires the worker mutex before the local job function object is destroyed. Destruction of its captured final owner enters ThreadPool::stop(), which attempts to acquire that same non-recursive mutex at line 104. The self-thread detach logic is never reached.

Reproduction:

  1. Let a subscription callback capture a shared_ptr<RedisAdapter> and block on a test barrier.
  2. While it is running, reset its ReaderHandle, then drop the external adapter owner.
  3. Release the callback and let it return normally.
  4. Its capture cleanup enters adapter destruction and hangs.

The full adapter case reproduced against private standalone Redis 7.0.12 on macOS. A pool-only case at #108 reproduces without Redis:

last owner released; destructor entered
FAIL: destructor deadlocked after callback returned
Pool-only reproducer

Save as /tmp/pool_lifetime.cpp and compile from the checked-out PR with c++ -std=c++17 -pthread -I . /tmp/pool_lifetime.cpp -o /tmp/pool_lifetime. The two-second failure exit deliberately terminates the deadlocked test process.

#include "ThreadPool.hpp"
#include <future>
#include <iostream>
#include <cstdlib>
int main() {
  std::promise<void> entered, release, destroyed;
  auto permit = release.get_future().share();
  auto done = destroyed.get_future();
  auto pool = std::shared_ptr<ThreadPool>(new ThreadPool(1), [&](ThreadPool* p) {
    std::cerr << "last owner released; destructor entered\n";
    delete p;
    destroyed.set_value();
  });
  pool->job("test", [keep = pool, &entered, permit] { entered.set_value(); permit.wait(); });
  entered.get_future().wait();
  pool.reset();
  release.set_value();
  if (done.wait_for(std::chrono::seconds(2)) != std::future_status::ready) {
    std::cerr << "FAIL: destructor deadlocked after callback returned\n";
    std::_Exit(2);
  }
  std::cout << "destructor completed\n";
}

Expected: the last owner can be released by normal callback capture cleanup without deadlock, self-join, or destruction of live worker storage.

A narrow experiment clearing the completed job while the worker mutex is still unlocked made the pool-only reproducer complete. No fix has been committed to the PR yet.

Fix PR: #108, pending implementation and validation. Add coverage for both explicit last-owner reset inside a callback and last-owner release when the completed callback object is destroyed. The existing lifecycle test covers only the former and currently passes despite this defect. Close only after the fix merges and the regressions pass.

Activity

  1. bigsamich commented on Sep 30, 2026

    @bigsamich
    Contributor

    Reproduced on Linux at #108, #109 and #111. Two additions for the acceptance criteria:

    1. The same line also deadlocks with no last owner. When a callback's captures own a sibling ReaderHandle and are destroyed after lk.lock(), the worker joins the reader thread while the reader waits for Worker::_mtx, and the whole adapter is wedged.
    2. The fix must sit after the catch handlers. Clearing job inside the try still deadlocks when the callback throws.

    job = nullptr; just before lk.lock() fixes all three cases: 600/600 adapter-level runs with 0 sanitizer reports. Details and a regression-test list are in the #108 thread on ThreadPool.hpp:93.

    Also relevant: removeReader()/removeGenericReader() destroy callbacks under _reader_mtx. That is a separate site of the same "user destructor under a library lock" problem, and this fix doesn't cover it (see the inline comment on #108).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions