Audit report AR-46

Reported privately in an independent security review, 2026-09-27.

Report date

2026-09-27

Kind

Bug

Status on master

Present

Proposed issue state

Open

Summary

If ThreadMap.makePool fails partway through creating a pool of worker threads, the client gets an error, but the threads created so far stay installed as the connection's pool, and a second makePool call is refused because a pool already exists. The report is accurate. It is the "no rollback on partial failure" part of AR-04, with a simple fix. The partial pool still serves calls, so the effect is a misleading error and a refused retry, not a crash or hang.

The report

makePool adds each worker to the connection's pool as soon as it is created. If creating a later worker throws, the request fails, but the earlier workers remain and the pool looks non-empty, so the client cannot retry on the same connection. A test that injects a failure before the third worker left two workers installed and showed the retry being rejected.

The report rates it low. A thread creation or allocation failure leaves that connection in a stale state until it is closed and reopened. The API is local and trusted, and no remote trigger, lasting corruption or funds effect is involved.

Proposed change

Build the pool in a local vector and install it only after every worker has started:

     const uint32_t count = context.getParams().getCount();
+    std::vector<Connection::PoolSlot> pool;
+    pool.reserve(count);
     for (uint32_t i = 0; i < count; ++i) {
 ...
-        m_connection.m_thread_pool.push_back({m_connection.m_threads.add(kj::mv(thread_server))});
+        pool.push_back({m_connection.m_threads.add(kj::mv(thread_server))});
     }
+    m_connection.m_thread_pool = std::move(pool);

If a worker fails, unwinding releases the ones already made and joins their threads, and the connection can try again. The test injects the failure, checks that no pool was installed, makes a one-thread pool, and runs a call on it. The report considered catching the exception and erasing the published entries instead, which would duplicate the teardown code.

Assessment

Accurate. On libmultiprocess master and in Bitcoin Core's copy, ProxyServer<ThreadMap>::makePool refuses a call when the pool is non-empty, then appends each new worker directly to the connection's pool. A failure leaves the earlier workers in place.

The realistic way to hit it is a request for more threads than the system allows: thread creation then throws, and the client is left with a pool of whatever was created. AR-04 showed that one request can create thousands of threads without error. Allocation failure is the other possible cause, but the Bitcoin Core node and GUI already abort on out-of-memory, and a failure at one specific step of the same loop aborts the process for a different reason (AR-47).

The partial pool works: calls without a named thread are spread over the workers that exist. So the client sees an error for a pool that is in fact installed and cannot replace it with a smaller one. That is a correctness problem, not a crash or hang. Only clients written outside libmultiprocess, such as Rust clients (mp#283 added it for them), call makePool; the libmultiprocess C++ client does not.

The fix is small and correct. It is the rollback that AR-04 already asks for, and it fits together with a limit on the requested count (AR-74).

Status

Present on master. Duplicate of part of AR-04; the fix belongs in the libmultiprocess issue planned there. No public issue or PR yet.

Reported privately in an independent security review on 2026-09-27.