Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Additional thread safety annotations for CNode/Peer members accessed via the message processing thread #24474

Closed
wants to merge 10 commits into from
Closed

Additional thread safety annotations for CNode/Peer members accessed via the message processing thread #24474

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
70a3324
net/net_processing: add missing thread safety annotations
ajtowns May 19, 2022
fca9fad
net: mark TransportSerializer/m_serializer as const
ajtowns Mar 4, 2022
021a96d
net: mark CNode unique_ptr members as const
ajtowns May 19, 2022
5df0bd1
net: note CNode members that are treated as const
ajtowns Mar 4, 2022
c8c4faa
net: add NetEventsInterface::g_mutex_msgproc_thread mutex
ajtowns Apr 20, 2021
b1af785
net: drop cs_sendProcessing
ajtowns Apr 20, 2021
8e75e74
[move-only] net: move NetEventsInterface before CNode
ajtowns Mar 4, 2022
49b72d2
net: add thread safety annotations for CNode/Peer members accessed on…
ajtowns Mar 4, 2022
3ca9617
net_processing: add thread safety annotations for Peer members access…
ajtowns May 19, 2022
49c3b28
net_processing: extra transactions data are accessed only via the msg…
ajtowns Apr 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
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".
  • Loading branch information
@ajtowns
ajtowns committed May 19, 2022
commit fca9fadd4b4b45f2476f1403c8e17057096adcda
3 changes: 2 additions & 1 deletion src/net.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,8 @@ CNetMessage V1TransportDeserializer::GetMessage(const std::chrono::microseconds
return msg;
}

void V1TransportSerializer::prepareForTransport(CSerializedNetMsg& msg, std::vector<unsigned char>& header) {
void V1TransportSerializer::prepareForTransport(CSerializedNetMsg& msg, std::vector<unsigned char>& header) const
{
// create dbl-sha256 checksum
uint256 hash = Hash(msg.data);

Expand Down
8 changes: 4 additions & 4 deletions src/net.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,13 +397,13 @@ class V1TransportDeserializer final : public TransportDeserializer
class TransportSerializer {
public:
// prepare message for transport (header construction, error-correction computation, payload encryption, etc.)
virtual void prepareForTransport(CSerializedNetMsg& msg, std::vector<unsigned char>& header) = 0;
virtual void prepareForTransport(CSerializedNetMsg& msg, std::vector<unsigned char>& header) const = 0;
virtual ~TransportSerializer() {}
};

class V1TransportSerializer : public TransportSerializer {
class V1TransportSerializer : public TransportSerializer {
public:
void prepareForTransport(CSerializedNetMsg& msg, std::vector<unsigned char>& header) override;
void prepareForTransport(CSerializedNetMsg& msg, std::vector<unsigned char>& header) const override;
};

/** Information about a peer */
Expand All @@ -414,7 +414,7 @@ class CNode

public:
std::unique_ptr<TransportDeserializer> m_deserializer;
std::unique_ptr<TransportSerializer> m_serializer;
std::unique_ptr<const TransportSerializer> m_serializer;

NetPermissionFlags m_permissionFlags{NetPermissionFlags::None};
std::atomic<ServiceFlags> nServices{NODE_NONE};
Expand Down