search

quickfixgo / quickfix

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

EventTimer.Stop panics if called more than once #581

Closed agukrapo closed 10 months ago

agukrapo commented 11 months ago

We use quickfix extensively in our systems and we are getting this panic several times per week across our services

panic: close of closed channel
goroutine 39832 [running]:
github.com/quickfixgo/quickfix/internal.(*EventTimer).Stop(...)
    /go/pkg/mod/github.com/quickfixgo/quickfix@v0.7.1-0.20230509031336-098031ef5379/internal/event_timer.go:48
github.com/quickfixgo/quickfix.(*session).run.func3()
    /go/pkg/mod/github.com/quickfixgo/quickfix@v0.7.1-0.20230509031336-098031ef5379/session.go:795 +0x4c
github.com/quickfixgo/quickfix.(*session).run(0xc00060cc00)
    /go/pkg/mod/github.com/quickfixgo/quickfix@v0.7.1-0.20230509031336-098031ef5379/session.go:823 +0x446
github.com/quickfixgo/quickfix.(*Initiator).handleConnection.func1()
    /go/pkg/mod/github.com/quickfixgo/quickfix@v0.7.1-0.20230509031336-098031ef5379/initiator.go:142 +0x25
created by github.com/quickfixgo/quickfix.(*Initiator).handleConnection in goroutine 39831
    /go/pkg/mod/github.com/quickfixgo/quickfix@v0.7.1-0.20230509031336-098031ef5379/initiator.go:141 +0xd3

A solution could be to make EventTimer.Stop idempotent:

func (t *EventTimer) Stop() {
    if t == nil {
        return
    }

    t.once.Do(func() {
        close(t.done)
    })

    t.wg.Wait()
}

This proposal is included in PR https://github.com/quickfixgo/quickfix/pull/580

melkanas commented 10 months ago

I had the same panic, managed to avoid this in my application by having a state of initiator and testing the state before initiator.Start() or initiator.Stop() to prevent calling them multiple times