Skip to content

Commit 66ddd64

Browse files
authored
fix race condition with context close and confirm at the same time on DeferredConfirmation. (#101)
* add failling concurrency test * add mutex to DeferredConfirmation to prevent race condition
1 parent 42c5149 commit 66ddd64

File tree

3 files changed

+46
-6
lines changed

3 files changed

+46
-6
lines changed

confirms.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,7 @@ func (d *deferredConfirmations) Confirm(confirmation Confirmation) {
144144
// we should never receive a confirmation for a tag that hasn't been published, but a test causes this to happen
145145
return
146146
}
147-
dc.confirmation = confirmation
148-
dc.cancel()
147+
dc.Confirm(confirmation.Ack)
149148
delete(d.confirmations, confirmation.DeliveryTag)
150149
}
151150

@@ -155,8 +154,7 @@ func (d *deferredConfirmations) ConfirmMultiple(confirmation Confirmation) {
155154

156155
for k, v := range d.confirmations {
157156
if k <= confirmation.DeliveryTag {
158-
v.confirmation = Confirmation{DeliveryTag: k, Ack: confirmation.Ack}
159-
v.cancel()
157+
v.Confirm(confirmation.Ack)
160158
delete(d.confirmations, k)
161159
}
162160
}
@@ -168,14 +166,25 @@ func (d *deferredConfirmations) Close() {
168166
defer d.m.Unlock()
169167

170168
for k, v := range d.confirmations {
171-
v.confirmation = Confirmation{DeliveryTag: k, Ack: false}
172-
v.cancel()
169+
v.Confirm(false)
173170
delete(d.confirmations, k)
174171
}
175172
}
176173

174+
// Confirm ack confirmation.
175+
func (d *DeferredConfirmation) Confirm(ack bool) {
176+
d.m.Lock()
177+
defer d.m.Unlock()
178+
179+
d.confirmation.Ack = ack
180+
d.cancel()
181+
}
182+
177183
// Waits for publisher confirmation. Returns true if server successfully received the publishing.
178184
func (d *DeferredConfirmation) Wait() bool {
179185
<-d.ctx.Done()
186+
187+
d.m.Lock()
188+
defer d.m.Unlock()
180189
return d.confirmation.Ack
181190
}

confirms_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,3 +301,32 @@ func TestDeferredConfirmationsContextTimeout(t *testing.T) {
301301
t.Fatal("expected to receive false for timeout confirmations, received true")
302302
}
303303
}
304+
305+
func TestDeferredConfirmationsContextConcurrency(t *testing.T) {
306+
ctx, cancel := context.WithCancel(context.Background())
307+
308+
dcs := newDeferredConfirmations()
309+
var wg sync.WaitGroup
310+
var result bool
311+
dc1 := dcs.Add(ctx, 1)
312+
dc2 := dcs.Add(ctx, 2)
313+
dc3 := dcs.Add(ctx, 3)
314+
wg.Add(1)
315+
316+
// wait confirmation
317+
go func() {
318+
result = !dc1.Wait() && !dc2.Wait() && !dc3.Wait()
319+
t.Logf("result: %v", result)
320+
wg.Done()
321+
}()
322+
323+
// confirm
324+
go func() {
325+
dcs.ConfirmMultiple(Confirmation{4, true})
326+
}()
327+
328+
// and cancel in //
329+
go cancel()
330+
331+
wg.Wait()
332+
}

types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"context"
1010
"fmt"
1111
"io"
12+
"sync"
1213
"time"
1314
)
1415

@@ -184,6 +185,7 @@ type Blocking struct {
184185
// allows users to directly correlate a publishing to a confirmation. These are
185186
// returned from PublishWithDeferredConfirm on Channels.
186187
type DeferredConfirmation struct {
188+
m sync.Mutex
187189
ctx context.Context
188190
cancel context.CancelFunc
189191
DeliveryTag uint64

0 commit comments

Comments
 (0)