quickfixgo / quickfix

The Go FIX Protocol Library :rocket:
https://www.quickfixgo.org/
Other
746 stars 290 forks source link

Multi dynamic session stop deadlock #531

Closed shipa988 closed 1 year ago

shipa988 commented 1 year ago

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 commented 1 year ago

532

ackleymi commented 1 year ago

Thanks! I will look at this shortly

tzachshabtay commented 8 months ago

@shipa988 this PR broke our initiator, started a discussion here: https://github.com/quickfixgo/quickfix/discussions/620