Skip to content

Commit ca9df87

Browse files
committed
dap: fix the race condition in the dap unit tests
The context used for serving the dap server was being canceled too early because it used defer which would initiate at the end of the function while every other cleanup function used `t.Cleanup` which executes in its own goroutine. One possible solution was to move the cancel to the cleanup, but the context being passed to serve and start doesn't make sense because if it ever does get canceled, it'll likely cause a similar race condition with `Stop`. This removes the context from the methods that were causing this issue in favor of just relying on the caller calling `Stop` when they are done with the adapter and server. This seems to have only affected tests and I don't believe it affected the actual dap command. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
1 parent c423bc7 commit ca9df87

4 files changed

Lines changed: 13 additions & 14 deletions

File tree

commands/dap.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package commands
22

33
import (
4-
"context"
54
"io"
65
"net"
76
"os"
@@ -75,7 +74,7 @@ type adapterProtocolDebugger struct {
7574
}
7675

7776
func (d *adapterProtocolDebugger) Start(printer *progress.Printer, opts *BuildOptions) error {
78-
cfg, err := d.Adapter.Start(context.Background(), d.conn)
77+
cfg, err := d.Adapter.Start(d.conn)
7978
if err != nil {
8079
return errors.Wrap(err, "debug adapter did not start")
8180
}

dap/adapter.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ func New[C LaunchConfig]() *Adapter[C] {
6767
return d
6868
}
6969

70-
func (d *Adapter[C]) Start(ctx context.Context, conn Conn) (C, error) {
71-
d.eg, _ = errgroup.WithContext(ctx)
70+
func (d *Adapter[C]) Start(conn Conn) (C, error) {
71+
d.eg, _ = errgroup.WithContext(context.Background())
7272
d.eg.Go(func() error {
73-
return d.srv.Serve(ctx, conn)
73+
return d.srv.Serve(conn)
7474
})
7575

7676
<-d.initialized

dap/adapter_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func TestLaunch(t *testing.T) {
2424

2525
eg, _ := errgroup.WithContext(ctx)
2626
eg.Go(func() error {
27-
_, err := adapter.Start(ctx, conn)
27+
_, err := adapter.Start(conn)
2828
assert.NoError(t, err)
2929
return nil
3030
})
@@ -57,13 +57,13 @@ func TestLaunch(t *testing.T) {
5757
// We should have received the initialized event.
5858
select {
5959
case <-initialized:
60-
default:
60+
case <-ctx.Done():
6161
assert.Fail(t, "did not receive initialized event")
6262
}
6363

6464
select {
6565
case <-configurationDone:
66-
case <-time.After(10 * time.Second):
66+
case <-ctx.Done():
6767
assert.Fail(t, "did not receive configurationDone response")
6868
}
6969
return nil
@@ -82,7 +82,7 @@ func TestSetBreakpoints(t *testing.T) {
8282

8383
eg, _ := errgroup.WithContext(ctx)
8484
eg.Go(func() error {
85-
_, err := adapter.Start(ctx, conn)
85+
_, err := adapter.Start(conn)
8686
assert.NoError(t, err)
8787
return nil
8888
})
@@ -118,7 +118,7 @@ func TestSetBreakpoints(t *testing.T) {
118118
// We should have received the initialized event.
119119
select {
120120
case <-initialized:
121-
default:
121+
case <-ctx.Done():
122122
assert.Fail(t, "did not receive initialized event")
123123
}
124124

@@ -127,7 +127,7 @@ func TestSetBreakpoints(t *testing.T) {
127127
assert.True(t, setBreakpointsResp.Success)
128128
assert.Len(t, setBreakpointsResp.Body.Breakpoints, 0)
129129
assert.NotNil(t, setBreakpointsResp.Body.Breakpoints, "breakpoints should be an empty array instead of null in the JSON")
130-
case <-time.After(10 * time.Second):
130+
case <-ctx.Done():
131131
assert.Fail(t, "did not receive setBreakpoints response")
132132
}
133133
return nil
@@ -249,11 +249,11 @@ func NewTestAdapter[C LaunchConfig](t *testing.T) (*Adapter[C], Conn, *daptest.C
249249
t.Cleanup(func() { clientConn.Close() })
250250

251251
adapter := New[C]()
252-
t.Cleanup(func() { adapter.Stop() })
253252

254253
client := daptest.NewClient(clientConn)
255254
t.Cleanup(func() { client.Close() })
256255

256+
t.Cleanup(func() { adapter.Stop() })
257257
return adapter, srvConn, client
258258
}
259259

dap/server.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@ func NewServer(h Handler) *Server {
3333
return &Server{h: h}
3434
}
3535

36-
func (s *Server) Serve(ctx context.Context, conn Conn) error {
36+
func (s *Server) Serve(conn Conn) error {
3737
writeCh := make(chan dap.Message)
3838
s.ch = writeCh
3939

40-
s.ctx, s.cancel = context.WithCancelCause(ctx)
40+
s.ctx, s.cancel = context.WithCancelCause(context.Background())
4141

4242
// Start an error group to handle server-initiated tasks.
4343
s.eg, _ = errgroup.WithContext(s.ctx)

0 commit comments

Comments
 (0)