diff --git a/field_map.go b/field_map.go index 4aac64b1d..d982cdb3b 100644 --- a/field_map.go +++ b/field_map.go @@ -196,7 +196,7 @@ func (m FieldMap) GetTime(tag Tag) (t time.Time, err MessageRejectError) { m.rwLock.RLock() defer m.rwLock.RUnlock() - bytes, err := m.GetBytes(tag) + bytes, err := m.getBytesNoLock(tag) if err != nil { return } diff --git a/field_map_test.go b/field_map_test.go index 0e9078734..ac58342a4 100644 --- a/field_map_test.go +++ b/field_map_test.go @@ -17,7 +17,9 @@ package quickfix import ( "bytes" + "sync" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -202,3 +204,46 @@ func TestFieldMap_Remove(t *testing.T) { assert.False(t, fMap.Has(1)) assert.True(t, fMap.Has(2)) } + +// TestFieldMap_GetTimeConcurrentWithSet verifies the fix guarding against +// recursive read-locking in GetTime. +func TestFieldMap_GetTimeConcurrentWithSet(t *testing.T) { + var fMap FieldMap + fMap.init() + fMap.SetField(Tag(1), FIXUTCTimestamp{Time: time.Now()}) + fMap.SetField(Tag(2), FIXString("blah")) + + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 50000; i++ { + fMap.SetField(Tag(2), FIXString("blah")) + } + }() + + for r := 0; r < 8; r++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < 50000; i++ { + if _, err := fMap.GetTime(Tag(1)); err != nil { + t.Error(err) + return + } + } + }() + } + + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + + select { + case <-done: + case <-time.After(60 * time.Second): + t.Fatal("deadlock: GetTime did not complete concurrently with writers") + } +}