Skip to content

Commit a17c092

Browse files
neildgopherbot
authored andcommittedMar 18, 2025
net/http: add onClose hook to fake net listener
Avoids a race condition: If we set an onClose hook on a conn created by a listener, then setting the hook can race with the connection closing. Change-Id: Ibadead3abbe4335d41f1e2cf84f4696fe98166b3 Reviewed-on: https://go-review.googlesource.com/c/go/+/658655 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com> Auto-Submit: Damien Neil <dneil@google.com>
1 parent 5916bc5 commit a17c092

File tree

2 files changed

+10
-7
lines changed

2 files changed

+10
-7
lines changed
 

‎src/net/http/netconn_test.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ type fakeNetListener struct {
3535
addr netip.AddrPort
3636
locPort uint16
3737

38-
onDial func() // called when making a new connection
38+
onDial func() // called when making a new connection
39+
onClose func(*fakeNetConn) // called when closing a connection
3940

4041
trackConns bool // set this to record all created conns
4142
conns []*fakeNetConn
@@ -65,6 +66,8 @@ func (li *fakeNetListener) connect() *fakeNetConn {
6566
locAddr := netip.AddrPortFrom(netip.AddrFrom4([4]byte{127, 0, 0, 1}), li.locPort)
6667
li.locPort++
6768
c0, c1 := fakeNetPipe(li.addr, locAddr)
69+
c0.onClose = li.onClose
70+
c1.onClose = li.onClose
6871
li.queue = append(li.queue, c0)
6972
if li.trackConns {
7073
li.conns = append(li.conns, c0)
@@ -124,7 +127,7 @@ type fakeNetConn struct {
124127
// peer is the other endpoint.
125128
peer *fakeNetConn
126129

127-
onClose func() // called when closing
130+
onClose func(*fakeNetConn) // called when closing
128131
}
129132

130133
// Read reads data from the connection.
@@ -167,7 +170,7 @@ func (c *fakeNetConn) IsClosedByPeer() bool {
167170
// Close closes the connection.
168171
func (c *fakeNetConn) Close() error {
169172
if c.onClose != nil {
170-
c.onClose()
173+
c.onClose(c)
171174
}
172175
// Local half of the conn is now closed.
173176
c.loc.lock()

‎src/net/http/transport_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -4250,6 +4250,10 @@ func testTransportIdleConnRacesRequest(t testing.TB, mode testMode) {
42504250
cst.li.onDial = func() {
42514251
<-dialc
42524252
}
4253+
closec := make(chan struct{})
4254+
cst.li.onClose = func(*fakeNetConn) {
4255+
<-closec
4256+
}
42534257
ctx, cancel := context.WithCancel(context.Background())
42544258
req1c := make(chan error)
42554259
go func() {
@@ -4279,10 +4283,6 @@ func testTransportIdleConnRacesRequest(t testing.TB, mode testMode) {
42794283
//
42804284
// First: Wait for IdleConnTimeout. The net.Conn.Close blocks.
42814285
synctest.Wait()
4282-
closec := make(chan struct{})
4283-
cst.li.conns[0].peer.onClose = func() {
4284-
<-closec
4285-
}
42864286
time.Sleep(timeout)
42874287
synctest.Wait()
42884288
// Make a request, which will use a new connection (since the existing one is closing).

0 commit comments

Comments
 (0)
Please sign in to comment.