Skip to content

Commit ed54c47

Browse files
committed
Fix purego BidirectionalConn buffer lifetime
1 parent 61f893e commit ed54c47

File tree

1 file changed

+57
-2
lines changed

1 file changed

+57
-2
lines changed

bidirectional_conn.go

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,18 @@ type BidirectionalConn struct {
2626
write chan struct{}
2727
headers map[string]string
2828

29+
// readSemaphore/writeSemaphore ensure that only one Read/Write operation is
30+
// in-flight at a time. This is required for Cronet and prevents reuse of
31+
// internal buffers while Cronet still holds pointers to them.
32+
readSemaphore chan struct{}
33+
writeSemaphore chan struct{}
34+
35+
// Cronet holds pointers to the Read/Write buffers asynchronously until the
36+
// corresponding callback fires. Never pass caller-provided buffers directly,
37+
// as they might reside on the Go stack and be moved during stack growth.
38+
readBuffer []byte
39+
writeBuffer []byte
40+
2941
// Buffer safety: when Read/Write return due to close/done, Cronet may
3042
// still hold the buffer. These channels are closed by callbacks to signal
3143
// it's safe to return. sync.Once ensures close happens exactly once.
@@ -45,9 +57,13 @@ func (e StreamEngine) CreateConn(readWaitHeaders bool, writeWaitHeaders bool) *B
4557
handshake: make(chan struct{}),
4658
read: make(chan int),
4759
write: make(chan struct{}),
60+
readSemaphore: make(chan struct{}, 1),
61+
writeSemaphore: make(chan struct{}, 1),
4862
readDone: make(chan struct{}, 1),
4963
writeDone: make(chan struct{}, 1),
5064
}
65+
conn.readSemaphore <- struct{}{}
66+
conn.writeSemaphore <- struct{}{}
5167
conn.stream = e.CreateStream(&bidirectionalHandler{BidirectionalConn: conn})
5268
return conn
5369
}
@@ -77,6 +93,18 @@ func (c *BidirectionalConn) Read(p []byte) (n int, err error) {
7793
return 0, net.ErrClosed
7894
default:
7995
}
96+
if len(p) == 0 {
97+
return 0, nil
98+
}
99+
100+
select {
101+
case <-c.close:
102+
return 0, net.ErrClosed
103+
case <-c.done:
104+
return 0, net.ErrClosed
105+
case <-c.readSemaphore:
106+
}
107+
defer func() { c.readSemaphore <- struct{}{} }()
80108

81109
if c.readWaitHeaders {
82110
select {
@@ -106,11 +134,21 @@ func (c *BidirectionalConn) Read(p []byte) (n int, err error) {
106134
default:
107135
}
108136

109-
c.stream.Read(p)
137+
if len(c.readBuffer) < len(p) {
138+
c.readBuffer = make([]byte, len(p))
139+
}
140+
readBuffer := c.readBuffer[:len(p)]
141+
c.stream.Read(readBuffer)
110142
c.access.Unlock()
111143

112144
select {
113145
case bytesRead := <-c.read:
146+
if bytesRead > len(p) {
147+
bytesRead = len(p)
148+
}
149+
if bytesRead > 0 {
150+
copy(p, readBuffer[:bytesRead])
151+
}
114152
return bytesRead, nil
115153
case <-c.done:
116154
// Wait for Cronet to finish using the buffer before returning.
@@ -133,6 +171,18 @@ func (c *BidirectionalConn) Write(p []byte) (n int, err error) {
133171
return 0, net.ErrClosed
134172
default:
135173
}
174+
if len(p) == 0 {
175+
return 0, nil
176+
}
177+
178+
select {
179+
case <-c.close:
180+
return 0, net.ErrClosed
181+
case <-c.done:
182+
return 0, net.ErrClosed
183+
case <-c.writeSemaphore:
184+
}
185+
defer func() { c.writeSemaphore <- struct{}{} }()
136186

137187
if c.writeWaitHeaders {
138188
select {
@@ -162,7 +212,12 @@ func (c *BidirectionalConn) Write(p []byte) (n int, err error) {
162212
default:
163213
}
164214

165-
c.stream.Write(p, false)
215+
if len(c.writeBuffer) < len(p) {
216+
c.writeBuffer = make([]byte, len(p))
217+
}
218+
writeBuffer := c.writeBuffer[:len(p)]
219+
copy(writeBuffer, p)
220+
c.stream.Write(writeBuffer, false)
166221
c.access.Unlock()
167222

168223
select {

0 commit comments

Comments
 (0)