kernel: Call log callbacks without holding the logger mutex #36766

pull ryanofsky wants to merge 1 commits into bitcoin:master from ryanofsky:pr/logcb changing 5 files +189 −29

ryanofsky commented at 10:29 PM on November 4, 2026: contributor
Problem: A kernel application whose log callback logs, or destroys a kernel object, hangs. The common way to hit this is a garbage-collected language binding: Python's garbage collector can run finalizers inside the callback, and a finalizer that destroys a chainstate manager flushes and logs (https://github.com/bitcoin/bitcoin/pull/34775#issuecomment-6278257737, #34374 (review)). The cause is that the kernel calls log callbacks while holding the logger's mutex, so any logging from inside a callback blocks on a mutex its own thread already holds.

Solution: This PR calls log callbacks without holding the mutex. The kernel API is unchanged, including support for multiple logging connections.

This is an alternative to the last commit of #34775, which fixes the same deadlock but only after limiting the process to one logging connection. Fixing the deadlock separately lets it merge without waiting on that API question. If #34775 is going to merge soon with its own fix, this PR can be closed.

Behavior changes for callers:
- Callbacks can run on several threads at the same time, so they must be thread-safe. Entries from different threads arrive in no particular order.
- A callback can log, and can create or destroy kernel objects and other logging connections. Destroying a callback's own connection from inside it aborts instead of hanging.
The change is in src/kernel/bitcoinkernel.cpp (about 45 lines) plus header docs and new test_kernel cases, which hang without the fix. Details are in the commit message.

kernel: Call log callbacks without holding the logger mutex

Release KernelLogger::m_mutex while a log callback runs, so a callback
can log, or destroy kernel objects that log, without deadlocking.
Previously Log() held m_mutex across every callback, so an entry logged
from inside a callback waited for the mutex its own thread held. A
garbage collector running finalizers inside a callback triggers this: a
finalizer that destroys a chainstate manager flushes and logs, and the
process hangs. See
https://github.com/bitcoin/bitcoin/pull/34775#issuecomment-6278257737
and https://github.com/bitcoin/bitcoin/pull/34374#discussion_r4030551866.
Creating or destroying another connection from inside a callback hung
the same way.

Multiple connections and the API are unchanged. Log() walks the list
of callbacks holding m_mutex only to step to the next one, and each
callback has a count of running invocations. Destroying a connection
marks its callback removed so no new invocations start, waits for the
count to reach zero, and then destroys the callback and its user_data
outside m_mutex, so it still waits for invocations on other threads.
The count is only decremented and notified under m_mutex, and destroy
takes m_mutex again before freeing the callback, so Log() never touches
a freed callback. Destroying a connection from inside its own callback
would wait for itself, so it asserts instead.

Behavior change: callbacks can run on several threads at the same time
and must be thread-safe, and entries from different threads have no
global order. The header documents this and which kernel calls are
safe from inside a callback. bitcoin-chainstate and the test logger now
write each line in one call so concurrent lines don't interleave.

New test_kernel cases, which all hang without this change: a callback
that logs and destroys a chainstate manager and context, a callback
that destroys another connection, and destroying a connection while
invocations run on other threads.

This change was written with Claude Opus 5.5.

08207f71a6

ryanofsky commented at 3:29 AM on November 5, 2026: contributor
Closing in favor of #36768, which fixes the same deadlock by delivering log entries through btcsignals::signal instead of this PR's hand-written callback list, providing a simpler implementation and a more straightforward nonblocking interface.

Both PRs keep the current API, including multiple connections, and run callbacks without holding the logger mutex, so a callback can log and destroy kernel objects. Both have tests showing it no longer hangs. The difference is what destroying a connection does while its callback is running on another thread:
- This PR waits for the call to return, like the current code and #34775. A thread waiting for its own call can't make progress, so a callback must not destroy a connection whose callback is running on the same thread. This PR asserts when a callback destroys its own connection.
- #36768 doesn't wait, like boost::signals2 and the node's other btcsignals users. The connection's user data is destroyed when the running call returns. A callback can destroy any connection, including its own. Callers that need to wait can do it from the user data's destroy callback.
#36768 also needs less kernel code (bitcoinkernel.cpp +42/-77 vs. +45/-16 here), since the signal implementation is shared.
ryanofsky closed this on Nov 5, 2026

Contributors

ryanofsky

Linked (view graph)

#34374 kernel: use struct-based logging and simplify logging interface #34775 kernel: allow only one global logging connection