Skip to content

Commit

Permalink
Fix a data race between Conn creation and its automatic closing
Browse files Browse the repository at this point in the history
`go test -race .` detects a data race when a Conn instance is automatically
closed when the provided context's deadline is missed.

Fix the data race by launching the corresponding goroutine only after all
Conn's fields had been initialized.
  • Loading branch information
ibazhitov authored and jsouthworth committed Jan 4, 2022
1 parent 8e438d3 commit e3cfec0
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 4 deletions.
9 changes: 5 additions & 4 deletions conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,6 @@ func newConn(tr transport, opts ...ConnOption) (*Conn, error) {
conn.ctx = context.Background()
}
conn.ctx, conn.cancelCtx = context.WithCancel(conn.ctx)
go func() {
<-conn.ctx.Done()
conn.Close()
}()

conn.calls = newCallTracker()
if conn.handler == nil {
Expand All @@ -302,6 +298,11 @@ func newConn(tr transport, opts ...ConnOption) (*Conn, error) {
conn.outHandler = &outputHandler{conn: conn}
conn.names = newNameTracker()
conn.busObj = conn.Object("org.freedesktop.DBus", "/org/freedesktop/DBus")

go func() {
<-conn.ctx.Done()
conn.Close()
}()
return conn, nil
}

Expand Down
22 changes: 22 additions & 0 deletions conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -798,3 +798,25 @@ func TestCancellingContextClosesConnection(t *testing.T) {
t.Errorf("expected connection to be closed, but got: %v", err)
}
}

// TestTimeoutContextClosesConnection checks that a Conn instance is closed after
// the passed context's deadline is missed.
// The test also checks that there's no data race between Conn creation and its
// automatic closing.
func TestTimeoutContextClosesConnection(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 0)
defer cancel()

bus, err := NewConn(rwc{}, WithContext(ctx))
if err != nil {
t.Fatal(err)
}

// wait until the connection is actually closed
time.Sleep(50 * time.Millisecond)

err = bus.BusObject().Call("org.freedesktop.DBus.Peer.Ping", 0).Store()
if err != ErrClosed {
t.Errorf("expected connection to be closed, but got: %v", err)
}
}

0 comments on commit e3cfec0

Please sign in to comment.