-
Notifications
You must be signed in to change notification settings - Fork 310
Block Sends when Resend Request is active #715
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
Block Sends when Resend Request is active #715
Conversation
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.
What is this mess? All you need is a single RWLock around the resend condition
@@ -230,6 +230,9 @@ func (state inSession) resendMessages(session *session, beginSeqNo, endSeqNo int | |||
return state.generateSequenceReset(session, beginSeqNo, endSeqNo+1, inReplyTo) | |||
} | |||
|
|||
session.resendMutex.Lock() |
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.
Please place a comment indicating that this lock must always be locked first before the sendMutex, otherwise there is potential for a deadlock, and that mutex is locked within the session.EnqueueBytesAndSend
method below
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.
@@ -302,6 +303,9 @@ func (s *session) sendInReplyTo(msg *Message, inReplyTo *Message) error { | |||
return s.queueForSend(msg) | |||
} | |||
|
|||
s.resendMutex.RLock() |
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.
Mirror other comment here as well
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.
@@ -40,7 +40,8 @@ type session struct { | |||
toSend [][]byte | |||
|
|||
// Mutex for access to toSend. | |||
sendMutex sync.Mutex | |||
sendMutex sync.Mutex | |||
resendMutex sync.RWMutex |
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.
Additional comment above this as well indicating the reason why this mutex exists in conjunction with the sendMutex and in which order they must be locked to avoid deadlocks
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.
No description provided.