-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[debugserver] Fix mutex scope in RNBRemote::CommDataReceived #131077
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.
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
Conversation
The mutex in RNBRemote::CommDataReceived protects m_rx_packets and should extend to the end of the function to cover the read where we check if the list is empty.
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesThe mutex in RNBRemote::CommDataReceived protects m_rx_packets and should extend to the end of the function to cover the read where we check if the list is empty. Full diff: https://github.com/llvm/llvm-project/pull/131077.diff 1 Files Affected:
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index 8a53094429aba..dd6da3db11814 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -1054,7 +1054,7 @@ rnb_err_t RNBRemote::HandleReceivedPacket(PacketEnum *type) {
void RNBRemote::CommDataReceived(const std::string &new_data) {
// DNBLogThreadedIf (LOG_RNB_REMOTE, "%8d RNBRemote::%s called",
// (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__);
- {
+
// Put the packet data into the buffer in a thread safe fashion
PThreadMutex::Locker locker(m_mutex);
@@ -1132,7 +1132,6 @@ void RNBRemote::CommDataReceived(const std::string &new_data) {
__FUNCTION__, data[idx]);
idx = idx + 1;
}
- }
}
if (!m_rx_packets.empty()) {
|
@@ -1054,7 +1054,7 @@ rnb_err_t RNBRemote::HandleReceivedPacket(PacketEnum *type) { | |||
void RNBRemote::CommDataReceived(const std::string &new_data) { | |||
// DNBLogThreadedIf (LOG_RNB_REMOTE, "%8d RNBRemote::%s called", | |||
// (uint32_t)m_comm.Timer().ElapsedMicroSeconds(true), __FUNCTION__); | |||
{ | |||
|
|||
// Put the packet data into the buffer in a thread safe fashion | |||
PThreadMutex::Locker locker(m_mutex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should you also fix the indentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I don't know why clang-format isn't kicking in...
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes looks correct to me.
…1077) The mutex in RNBRemote::CommDataReceived protects m_rx_packets and should extend to the end of the function to cover the read where we check if the list is empty.
The mutex in RNBRemote::CommDataReceived protects m_rx_packets and should extend to the end of the function to cover the read where we check if the list is empty.