Skip to content
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

Multi dynamic session stop deadlock #531

Merged

Conversation

shipa988
Copy link

We have floating deadlocks when stop the Acceptor. Firstly deadlocks appear in acceptor.go:354. This connected with fact that session.stop() func is called not once. It can be called from 3 places in acceptor (but static, not dynamic session, could be stopped only once) : acceptor.go:354 and acceptor.go:323 and acceptor.go:129!
image
That is why after first call session.stop() we write s.admin <- stopReq{} then this s.admin chan processed here in select statement session.go:772 while for !s.Stopped(). and after processing here in session.go:732 we switch s.Stopped to false and quit from select loop and have never listen s.admin more! Second call session.stop() and writing to s.admin calls deadlock((
My proposal is add sync.Once() inside call of session.stop() and write s.admin <- stopReq{} once! Of course we can think more and try to leave only one call of session.stop() such in static session, but sync.Once() is not so bad decision, IMHO)

@shipa988
Copy link
Author

#532

@shipa988 shipa988 force-pushed the multi-dynamic-session-stop-deadlock branch from 03c1c5a to bc299df Compare December 21, 2022 16:27
@ackleymi
Copy link
Member

Thanks! I will look at this shortly

@ackleymi ackleymi merged commit c83230c into quickfixgo:main Dec 24, 2022
@tzachshabtay
Copy link

@shipa988 this PR broke our initiator, started a discussion here: #620

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants