quickfixgo / quickfix

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

Refresh can reset the session #562

Open Gilthoniel opened 1 year ago

Gilthoniel commented 1 year ago

Hi,

Before going deeper into a fix, I'd like your opinion on this: if a refresh is occurring while messages are still attempted to be sent, it can happen that the session is reset. See:

    if session.RefreshOnLogon {
        if err := session.store.Refresh(); err != nil {
            session.logError(err)
            return
        }
    }
    session.log.OnEvent("Sending logon request")
    if err := session.sendLogon(); err != nil {
        session.logError(err)
        return
    }
// queueForSend will validate, persist, and queue the message for send
func (s *session) queueForSend(msg *Message) error {
    s.sendMutex.Lock()
    defer s.sendMutex.Unlock()

The store is usually protected against concurrent calls with the sendMutex of the session, but when a refresh is happening, the store can be called concurrently with queueForSend and store.Refresh(). Knowing that the refresh first reset the cache then goes into the database to populate it again, it can happen that sending resets the outgoing sequence number to 1.