refactor: replace RecursiveMutex `cs_sendProcessing` with Mutex #25597

pull theStack wants to merge 1 commits into bitcoin:master from theStack:202207-refactor-change_send_processing_recursive_mutex_to_mutex changing 1 files +1 −1

theStack commented at 3:24 PM on July 12, 2022: contributor

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

https://github.com/bitcoin/bitcoin/blob/7d258ee8bc3bff439881064dd6234f7b04982783/src/net.cpp#L1998-L2001

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.)

refactor: replace RecursiveMutex `cs_sendProcessing` with Mutex

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.

3649d140ca

maflcko commented at 3:29 PM on July 12, 2022: member

See also https://github.com/bitcoin/bitcoin/pull/24474/commits/b1af785346cd2c932fd1310df10f757ba1346c2b (https://github.com/bitcoin/bitcoin/pull/24474/commits/b1af785346cd2c932fd1310df10f757ba1346c2b)
DrahtBot added the label Refactoring on Jul 12, 2022
ajtowns commented at 8:38 AM on September 9, 2022: contributor

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

Or #26036 more broadly
DrahtBot commented at 7:44 PM on September 13, 2022: 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 added the label Needs rebase on Sep 20, 2022
DrahtBot commented at 1:45 PM on September 20, 2022: contributor



🐙 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>
achow101 commented at 7:44 PM on October 12, 2022: member

Are you still working on this?
glozow commented at 7:47 PM on October 12, 2022: member

This mutex no longer exists since #26036 / bf12abe4542f2cf658516ea7e7fbbff6864c2208. Closing.
glozow closed this on Oct 12, 2022
bitcoin locked this on Oct 12, 2023

Contributors

Labels

Refactoring Needs rebase

Linked (view graph)

#19303 Replace all of the RecursiveMutex instances with the Mutex ones

refactor: replace RecursiveMutex `cs_sendProcessing` with Mutex #25597

Conflicts