Skip to content

Commit 8e68ff9

Browse files
authored
Add extra disconnect safety (minio#19022)
Fix reported races that are actually synchronized by network calls. But this should add some extra safety for untimely disconnects. Race reported: ``` WARNING: DATA RACE Read at 0x00c00171c9c0 by goroutine 214: github.com/minio/minio/internal/grid.(*muxClient).addResponse() e:/gopath/src/github.com/minio/minio/internal/grid/muxclient.go:519 +0x111 github.com/minio/minio/internal/grid.(*muxClient).error() e:/gopath/src/github.com/minio/minio/internal/grid/muxclient.go:470 +0x21d github.com/minio/minio/internal/grid.(*Connection).handleDisconnectClientMux() e:/gopath/src/github.com/minio/minio/internal/grid/connection.go:1391 +0x15b github.com/minio/minio/internal/grid.(*Connection).handleMsg() e:/gopath/src/github.com/minio/minio/internal/grid/connection.go:1190 +0x1ab github.com/minio/minio/internal/grid.(*Connection).handleMessages.func1() e:/gopath/src/github.com/minio/minio/internal/grid/connection.go:981 +0x610 Previous write at 0x00c00171c9c0 by goroutine 1081: github.com/minio/minio/internal/grid.(*muxClient).roundtrip() e:/gopath/src/github.com/minio/minio/internal/grid/muxclient.go:94 +0x324 github.com/minio/minio/internal/grid.(*muxClient).traceRoundtrip() e:/gopath/src/github.com/minio/minio/internal/grid/trace.go:74 +0x10e4 github.com/minio/minio/internal/grid.(*Subroute).Request() e:/gopath/src/github.com/minio/minio/internal/grid/connection.go:366 +0x230 github.com/minio/minio/internal/grid.(*SingleHandler[go.shape.*github.com/minio/minio/cmd.DiskInfoOptions,go.shape.*github.com/minio/minio/cmd.DiskInfo]).Call() e:/gopath/src/github.com/minio/minio/internal/grid/handlers.go:554 +0x3fd github.com/minio/minio/cmd.(*storageRESTClient).DiskInfo() e:/gopath/src/github.com/minio/minio/cmd/storage-rest-client.go:314 +0x270 github.com/minio/minio/cmd.erasureObjects.getOnlineDisksWithHealingAndInfo.func1() e:/gopath/src/github.com/minio/minio/cmd/erasure.go:293 +0x171 ``` This read will always happen after the write, since there is a network call in between. However a disconnect could come in while we are setting up the call, so we protect against that with extra checks.
1 parent 62761a2 commit 8e68ff9

File tree

2 files changed

+17
-4
lines changed

2 files changed

+17
-4
lines changed

internal/grid/debugmsg_string.go

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/grid/muxclient.go

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,13 @@ func (m *muxClient) roundtrip(h HandlerID, req []byte) ([]byte, error) {
9191
msg.Flags |= FlagSubroute
9292
}
9393
ch := make(chan Response, 1)
94+
m.respMu.Lock()
95+
if m.closed {
96+
m.respMu.Unlock()
97+
return nil, ErrDisconnected
98+
}
9499
m.respWait = ch
100+
m.respMu.Unlock()
95101
ctx := m.ctx
96102

97103
// Add deadline if none.
@@ -101,8 +107,8 @@ func (m *muxClient) roundtrip(h HandlerID, req []byte) ([]byte, error) {
101107
ctx, cancel = context.WithTimeout(ctx, defaultSingleRequestTimeout)
102108
defer cancel()
103109
}
104-
// Send... (no need for lock yet)
105-
if err := m.sendLocked(msg); err != nil {
110+
// Send request
111+
if err := m.send(msg); err != nil {
106112
return nil, err
107113
}
108114
if debugReqs {
@@ -215,7 +221,13 @@ func (m *muxClient) RequestStream(h HandlerID, payload []byte, requests chan []b
215221
return nil, errors.New("RequestStream: responses channel is nil")
216222
}
217223
m.init = true
224+
m.respMu.Lock()
225+
if m.closed {
226+
m.respMu.Unlock()
227+
return nil, ErrDisconnected
228+
}
218229
m.respWait = responses // Route directly to output.
230+
m.respMu.Unlock()
219231

220232
// Try to grab an initial block.
221233
m.singleResp = false

0 commit comments

Comments
 (0)