Skip to content

Conversation

theStack
Copy link
Contributor

The mutex cs_sendProcessing is currently only acquired at one location in the code:

bitcoin/src/net.cpp

Lines 1998 to 2001 in 7d258ee

{
LOCK(pnode->cs_sendProcessing);
m_msgproc->SendMessages(pnode);
}

apparently with the intention to execute the method for sending messages (NetEventsInterface::SendMessages(...)) in serial order, in the case that we ever had more than one message handling thread. With no possibility to re-acquire the mutex in the same thread, there is no reason for it to be a RecursiveMutex, so we can change it to an ordinary Mutex instead.

(An alternative would be to just get rid of the mutex, as it currently doesn't serve any purpose, as long as we only have one message handling thread.)

This mutex is currently only acquired at one location in the code,
apparently with the intention to execute the method for sending messages
(`NetEventsInterface::SendMessages(...)`) in serial order, in the case
that we ever had more than one message thread. With no possibility to
re-acquire the mutex in the same thread, there is no reason for it to be
a RecursiveMutex, so we can change it to an ordinary Mutex instead.
@maflcko
Copy link
Member

maflcko commented Jul 12, 2022

See also b1af785 (https://github.com/bitcoin/bitcoin/pull/24474/commits/b1af785346cd2c932fd1310df10f757ba1346c2b)

@ajtowns
Copy link
Contributor

ajtowns commented Sep 9, 2022

See also b1af785 (https://github.com/bitcoin/bitcoin/pull/24474/commits/b1af785346cd2c932fd1310df10f757ba1346c2b)

Or #26036 more broadly

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #26036 (net: add NetEventsInterface::g_msgproc_mutex by ajtowns)

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.

@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@achow101
Copy link
Member

Are you still working on this?

@glozow
Copy link
Member

glozow commented Oct 12, 2022

This mutex no longer exists since #26036 / bf12abe. Closing.

@glozow glozow closed this Oct 12, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Oct 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants