ajtowns
commented at 7:14 PM on March 4, 2022:
contributor
Documents why the existing accesses of elements in net.h's CNode and net_processing.cpp's Peer objects is thread safe, generally by making it so that the compiler will complain if the non-safe accesses are added. Only leaves CNode's m_permissionFlags and m_prefer_evict unenforced by the compiler.
Adds a global mutex NetEventsInterface::g_mutex_msgproc_thread which is used to document the contract between net_processing and net that ProcessMessages and SendMessages are only invoked from a single thread, and which replaces the per-peer cs_sendProcessing mutex.
DrahtBot added the label P2P on Mar 4, 2022
ajtowns
commented at 7:39 PM on March 4, 2022:
contributor
I've pulled the m_mutex_message_handling change out of #21527 since it seems to have been the sticking point and refined it so that it is used to document all the un-annotated net/net_processing CNode/Peer variables in regards to thread-safety. The idea is this PR should just be converting the implicit assumptions we're already making to explicit ones.
DrahtBot
commented at 8:35 PM on March 4, 2022:
contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
Reviewers, this pull request conflicts with the following ones:
#25168 (refactor: Avoid passing params where not needed by MarcoFalke)
#24970 (net processing: Move cleanSubVer, fPreferredDownload and nLocalHostNonce to Peer by jnewbery)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
net: mark TransportSerializer/m_serializer as const
The (V1)TransportSerializer instance CNode::m_serializer is used from
multiple threads via PushMessage without protection by a mutex. This
is only thread safe because the class does not have any mutable state,
so document that by marking the methods and the object as "const".
fca9fadd4b
net: mark CNode unique_ptr members as const
Dereferencing a unique_ptr is not necessarily thread safe. The reason
these are safe is because their values are set at construction and do
not change later; so mark them as const and set them via the initializer
list to guarantee that.
021a96d41b
net: note CNode members that are treated as const
m_permissionFlags and m_prefer_evict are treated as const -- they're
only set immediately after construction before any other thread has
access to the object, and not changed again afterwards. As such they
don't need to be marked atomic or guarded by a mutex; though it would
probably be better to actually mark them as const...
There are many cases where we asssume message processing is
single-threaded in order for how we access node-related memory to be
safe. Add an explicit mutex that we can use to document this, which allows
the compiler to catch any cases where we try to access that memory from
other threads and break that assumption.
c8c4faac48
net: drop cs_sendProcessing
SendMessages() is now protected g_mutex_msgproc_thread; so this additional
per-node mutex is redundant.
b1af785346
[move-only] net: move NetEventsInterface before CNode
This allows CNode members to be marked as guarded by the
g_mutex_msgproc_thread mutex.
8e75e74e6a
net: add thread safety annotations for CNode/Peer members accessed only via the msgproc thread49b72d235c
net_processing: add thread safety annotations for Peer members accessed only via the msgproc thread3ca9617d45
net_processing: extra transactions data are accessed only via the msgproc thread
Previously vExtraTxnForCompact and vExtraTxnForCompactIt were protected
by g_cs_orphans; protect them by g_mutex_msgproc_thread instead, as they
are only used during message processing.
49c3b28ab4
ajtowns force-pushed on May 19, 2022
ajtowns
commented at 8:32 PM on May 19, 2022:
contributor
DrahtBot added the label Needs rebase on May 20, 2022
DrahtBot
commented at 1:40 PM on May 20, 2022:
contributor
<!--cf906140f33d8803c4a75a2196329ecb-->
🐙 This pull request conflicts with the target branch and needs rebase.
<sub>Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".</sub>
ajtowns renamed this: Additional thread safety annotations for CNode/Peer Additional thread safety annotations for CNode/Peer members accessed via the message processing thread on May 20, 2022
ajtowns
commented at 6:53 AM on August 29, 2022:
contributor
Closing this. If you'd like it resurrected please help #25174 make progress.
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:53 UTC
