DbgTransportSession: delete message copy when not queued #50550
Conversation
|
Tagging subscribers to this area: @tommcdon Issue DetailsThis tackles an issue reported by Coverity about
|
| @@ -628,6 +628,13 @@ HRESULT DbgTransportSession::SendMessage(Message *pMessage, bool fWaitsForReply) | |||
| // queue). | |||
| pMessage->m_sHeader.m_dwLastSeenId = m_dwLastMessageIdSeen; | |||
|
|
|||
| // Check the session state. | |||
| if (m_eState == SS_Closed) | |||
There was a problem hiding this comment.
I've moved this check up, so we don't create a message copy to delete it.
| // Check the session state. | ||
| if (m_eState == SS_Closed) | ||
| // If the state is SS_Open we can send the message now. | ||
| if (m_eState == SS_Open) |
There was a problem hiding this comment.
This send was moved before the queuing of the message. I assume this is not a problem because we're holding a state lock.
|
|
||
| // If the state is SS_Open we can send the message now. | ||
| if (m_eState == SS_Open) | ||
| else |
There was a problem hiding this comment.
At this point, we're either adding the message to the queue, where the queue reader is responsible for freeing.
Or else we delete when we've allocated a copy.
|
@janvorli can you take a look? |
There was a problem hiding this comment.
It looks good, but I'd like @mikem8361 to review it to make sure there is no subtle problem.
|
@mikem8361 there are CI failures, can you see if they are related? If not, this should be good to merge. |
|
/azp run runtime |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hello @hoyosjs! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit 
ghost
locked as resolved and limited conversation to collaborators
This tackles an issue reported by Coverity about
pMessageCopygoing out-of-scope without being freed.