Cleanup code

Author: nekohasekai

.github/workflows/naive-build.yml

@@ -4,7 +4,7 @@ on:
   workflow_dispatch:
     inputs:
       version:
-        description: 'Release version (e.g., v1.0.0). Creates GitHub Release draft when provided.'
+        description: 'Release tag'
         type: string
         required: false
   push:
@@ -29,7 +29,7 @@ jobs:
           submodules: 'recursive'
       - uses: actions/setup-go@v5
         with:
-          go-version: ^1.22
+          go-version: ^1.25
       - name: Get naiveproxy commit
         id: naive
         run: echo "commit=$(git -C naiveproxy rev-parse HEAD)" >> $GITHUB_OUTPUT

.golangci.yml

@@ -6,7 +6,6 @@ linters:
   enable:
     - govet
     - ineffassign
-    - paralleltest
     - staticcheck
   settings:
     staticcheck:

Makefile

@@ -5,6 +5,9 @@ build:
 	go run -v ./cmd/build-naive build $(TARGET_FLAG)
 	go run -v ./cmd/build-naive package --local $(TARGET_FLAG)
 
+generate_net_errors:
+	go run ./cmd/build-naive generate-net-errors
+
 test: make
 	go test -v .
 

bidirectional_conn.go

@@ -9,11 +9,10 @@ import (
 	"time"
 )
 
-// BidirectionalConn is a wrapper from BidirectionalStream to net.Conn
 type BidirectionalConn struct {
-	stream      BidirectionalStream
-	cancelOnce  sync.Once // Ensures Cancel is called at most once
-	destroyOnce sync.Once // Ensures Destroy is called at most once
+	stream           BidirectionalStream
+	cancelOnce       sync.Once // Ensures Cancel is called at most once
+	destroyOnce      sync.Once // Ensures Destroy is called at most once
 	readWaitHeaders  bool
 	writeWaitHeaders bool
 	access           sync.Mutex
@@ -25,26 +24,14 @@ type BidirectionalConn struct {
 	read             chan int
 	write            chan struct{}
 	headers          map[string]string
-
-	// readSemaphore/writeSemaphore ensure that only one Read/Write operation is
-	// in-flight at a time. This is required for Cronet and prevents reuse of
-	// internal buffers while Cronet still holds pointers to them.
-	readSemaphore  chan struct{}
-	writeSemaphore chan struct{}
-
-	// Cronet holds pointers to the Read/Write buffers asynchronously until the
-	// corresponding callback fires. Never pass caller-provided buffers directly,
-	// as they might reside on the Go stack and be moved during stack growth.
-	readBuffer  []byte
-	writeBuffer []byte
-
-	// Buffer safety: when Read/Write return due to close/done, Cronet may
-	// still hold the buffer. These channels are closed by callbacks to signal
-	// it's safe to return. sync.Once ensures close happens exactly once.
-	readDone      chan struct{}
-	writeDone     chan struct{}
-	readDoneOnce  sync.Once
-	writeDoneOnce sync.Once
+	readSemaphore    chan struct{}
+	writeSemaphore   chan struct{}
+	readBuffer       []byte
+	writeBuffer      []byte
+	readDone         chan struct{}
+	writeDone        chan struct{}
+	readDoneOnce     sync.Once
+	writeDoneOnce    sync.Once
 }
 
 func (e StreamEngine) CreateConn(readWaitHeaders bool, writeWaitHeaders bool) *BidirectionalConn {
@@ -84,7 +71,6 @@ func (c *BidirectionalConn) Start(method string, url string, headers map[string]
 	return nil
 }
 
-// Read implements io.Reader
 func (c *BidirectionalConn) Read(p []byte) (n int, err error) {
 	select {
 	case <-c.close:
@@ -151,18 +137,14 @@ func (c *BidirectionalConn) Read(p []byte) (n int, err error) {
 		}
 		return bytesRead, nil
 	case <-c.done:
-		// Wait for Cronet to finish using the buffer before returning.
-		// Callbacks will close readDone when done.
 		<-c.readDone
 		return 0, c.err
 	case <-c.close:
-		// Close() was called. Wait for OnCanceled to signal buffer safety.
 		<-c.readDone
 		return 0, net.ErrClosed
 	}
 }
 
-// Write implements io.Writer
 func (c *BidirectionalConn) Write(p []byte) (n int, err error) {
 	select {
 	case <-c.close:
@@ -224,28 +206,22 @@ func (c *BidirectionalConn) Write(p []byte) (n int, err error) {
 	case <-c.write:
 		return len(p), nil
 	case <-c.done:
-		// Wait for Cronet to finish using the buffer before returning.
-		// Callbacks will close writeDone when done.
 		<-c.writeDone
 		return 0, c.err
 	case <-c.close:
-		// Close() was called. Wait for OnCanceled to signal buffer safety.
 		<-c.writeDone
 		return 0, net.ErrClosed
 	}
 }
 
-// Done implements context.Context
 func (c *BidirectionalConn) Done() <-chan struct{} {
 	return c.done
 }
 
-// Err implements context.Context
 func (c *BidirectionalConn) Err() error {
 	return c.err
 }
 
-// Close implements io.Closer
 func (c *BidirectionalConn) Close() error {
 	c.access.Lock()
 
@@ -276,27 +252,22 @@ func (c *BidirectionalConn) signalWriteDone() {
 	c.writeDoneOnce.Do(func() { close(c.writeDone) })
 }
 
-// LocalAddr implements net.Conn
 func (c *BidirectionalConn) LocalAddr() net.Addr {
 	return nil
 }
 
-// RemoteAddr implements net.Conn
 func (c *BidirectionalConn) RemoteAddr() net.Addr {
 	return nil
 }
 
-// SetDeadline implements net.Conn
 func (c *BidirectionalConn) SetDeadline(t time.Time) error {
 	return os.ErrInvalid
 }
 
-// SetReadDeadline implements net.Conn
 func (c *BidirectionalConn) SetReadDeadline(t time.Time) error {
 	return os.ErrInvalid
 }
 
-// SetWriteDeadline implements net.Conn
 func (c *BidirectionalConn) SetWriteDeadline(t time.Time) error {
 	return os.ErrInvalid
 }

bidirectional_stream_cgo.go

@@ -81,8 +81,7 @@ func (c BidirectionalStream) Start(method string, url string, headers map[string
 	if headerLen > 0 {
 		cHeadersPtr := C.malloc(C.size_t(int(C.sizeof_struct_bidirectional_stream_header) * headerLen))
 		defer C.free(cHeadersPtr)
-		var cType *C.bidirectional_stream_header
-		cType = (*C.bidirectional_stream_header)(cHeadersPtr)
+		cType := (*C.bidirectional_stream_header)(cHeadersPtr)
 		cHeaders := unsafe.Slice(cType, headerLen)
 		var index int
 		for key, value := range headers {

bidirectional_stream_impl_cgo.go

@@ -45,7 +45,7 @@ func cBidirectionalStreamToGo(stream *C.bidirectional_stream) BidirectionalStrea
 func cronetBidirectionalStreamOnStreamReady(stream *C.bidirectional_stream) {
 	callback := instanceOfBidirectionalStream(stream)
 	if callback == nil {
-		return // Post-destroy callback, silently ignore
+		return
 	}
 	callback.OnStreamReady(cBidirectionalStreamToGo(stream))
 }
@@ -54,7 +54,7 @@ func cronetBidirectionalStreamOnStreamReady(stream *C.bidirectional_stream) {
 func cronetBidirectionalStreamOnResponseHeadersReceived(stream *C.bidirectional_stream, headers *C.bidirectional_stream_header_array, negotiatedProtocol *C.char) {
 	callback := instanceOfBidirectionalStream(stream)
 	if callback == nil {
-		return // Post-destroy callback, silently ignore
+		return
 	}
 	var headerMap map[string]string
 	if headers != nil && headers.count > 0 {
@@ -75,7 +75,7 @@ func cronetBidirectionalStreamOnResponseHeadersReceived(stream *C.bidirectional_
 func cronetBidirectionalStreamOnReadCompleted(stream *C.bidirectional_stream, data *C.char, bytesRead C.int) {
 	callback := instanceOfBidirectionalStream(stream)
 	if callback == nil {
-		return // Post-destroy callback, silently ignore
+		return
 	}
 	callback.OnReadCompleted(cBidirectionalStreamToGo(stream), int(bytesRead))
 }
@@ -84,7 +84,7 @@ func cronetBidirectionalStreamOnReadCompleted(stream *C.bidirectional_stream, da
 func cronetBidirectionalStreamOnWriteCompleted(stream *C.bidirectional_stream, data *C.char) {
 	callback := instanceOfBidirectionalStream(stream)
 	if callback == nil {
-		return // Post-destroy callback, silently ignore
+		return
 	}
 	callback.OnWriteCompleted(cBidirectionalStreamToGo(stream))
 }
@@ -93,7 +93,7 @@ func cronetBidirectionalStreamOnWriteCompleted(stream *C.bidirectional_stream, d
 func cronetBidirectionalStreamOnResponseTrailersReceived(stream *C.bidirectional_stream, trailers *C.bidirectional_stream_header_array) {
 	callback := instanceOfBidirectionalStream(stream)
 	if callback == nil {
-		return // Post-destroy callback, silently ignore
+		return
 	}
 	var trailersMap map[string]string
 	if trailers != nil && trailers.count > 0 {
@@ -114,31 +114,28 @@ func cronetBidirectionalStreamOnResponseTrailersReceived(stream *C.bidirectional
 func cronetBidirectionalStreamOnSucceed(stream *C.bidirectional_stream) {
 	callback := instanceOfBidirectionalStream(stream)
 	if callback == nil {
-		return // Post-destroy callback, silently ignore
+		return
 	}
 	callback.OnSucceeded(cBidirectionalStreamToGo(stream))
-	// Terminal callback - safe to cleanup
 	cleanupBidirectionalStream(uintptr(unsafe.Pointer(stream)))
 }
 
 //export cronetBidirectionalStreamOnFailed
 func cronetBidirectionalStreamOnFailed(stream *C.bidirectional_stream, netError C.int) {
 	callback := instanceOfBidirectionalStream(stream)
 	if callback == nil {
-		return // Post-destroy callback, silently ignore
+		return
 	}
 	callback.OnFailed(cBidirectionalStreamToGo(stream), int(netError))
-	// Terminal callback - safe to cleanup
 	cleanupBidirectionalStream(uintptr(unsafe.Pointer(stream)))
 }
 
 //export cronetBidirectionalStreamOnCanceled
 func cronetBidirectionalStreamOnCanceled(stream *C.bidirectional_stream) {
 	callback := instanceOfBidirectionalStream(stream)
 	if callback == nil {
-		return // Post-destroy callback, silently ignore
+		return
 	}
 	callback.OnCanceled(cBidirectionalStreamToGo(stream))
-	// Terminal callback - safe to cleanup
 	cleanupBidirectionalStream(uintptr(unsafe.Pointer(stream)))
 }

bidirectional_stream_impl_purego.go

@@ -10,8 +10,6 @@ import (
 	"github.com/ebitengine/purego"
 )
 
-// bidirectionalStreamCallbackStruct mirrors the C struct bidirectional_stream_callback.
-// It contains 8 function pointers in the same order as the C struct.
 type bidirectionalStreamCallbackStruct struct {
 	onStreamReady              uintptr
 	onResponseHeadersReceived  uintptr
@@ -23,14 +21,12 @@ type bidirectionalStreamCallbackStruct struct {
 	onCanceled                 uintptr
 }
 
-// bidirectionalStreamHeaderArray mirrors the C struct bidirectional_stream_header_array
 type bidirectionalStreamHeaderArray struct {
 	count    uintptr
 	capacity uintptr
 	headers  uintptr
 }
 
-// bidirectionalStreamHeader mirrors the C struct bidirectional_stream_header
 type bidirectionalStreamHeader struct {
 	key   uintptr // const char*
 	value uintptr // const char*
@@ -52,7 +48,7 @@ func init() {
 func bsOnStreamReadyCallback(stream uintptr) uintptr {
 	cb := instanceOfBidirectionalStreamCallback(stream)
 	if cb == nil {
-		return 0 // Post-destroy callback, silently ignore
+		return 0
 	}
 	cb.OnStreamReady(BidirectionalStream{stream})
 	return 0
@@ -61,7 +57,7 @@ func bsOnStreamReadyCallback(stream uintptr) uintptr {
 func bsOnResponseHeadersReceivedCallback(stream, headers, negotiatedProtocol uintptr) uintptr {
 	cb := instanceOfBidirectionalStreamCallback(stream)
 	if cb == nil {
-		return 0 // Post-destroy callback, silently ignore
+		return 0
 	}
 	headerMap := parseHeaderArray(headers)
 	cb.OnResponseHeadersReceived(BidirectionalStream{stream}, headerMap, cronet.GoString(negotiatedProtocol))
@@ -71,7 +67,7 @@ func bsOnResponseHeadersReceivedCallback(stream, headers, negotiatedProtocol uin
 func bsOnReadCompletedCallback(stream, data uintptr, bytesRead int32) uintptr {
 	cb := instanceOfBidirectionalStreamCallback(stream)
 	if cb == nil {
-		return 0 // Post-destroy callback, silently ignore
+		return 0
 	}
 	cb.OnReadCompleted(BidirectionalStream{stream}, int(bytesRead))
 	return 0
@@ -80,7 +76,7 @@ func bsOnReadCompletedCallback(stream, data uintptr, bytesRead int32) uintptr {
 func bsOnWriteCompletedCallback(stream, data uintptr) uintptr {
 	cb := instanceOfBidirectionalStreamCallback(stream)
 	if cb == nil {
-		return 0 // Post-destroy callback, silently ignore
+		return 0
 	}
 	cb.OnWriteCompleted(BidirectionalStream{stream})
 	return 0
@@ -89,7 +85,7 @@ func bsOnWriteCompletedCallback(stream, data uintptr) uintptr {
 func bsOnResponseTrailersReceivedCallback(stream, trailers uintptr) uintptr {
 	cb := instanceOfBidirectionalStreamCallback(stream)
 	if cb == nil {
-		return 0 // Post-destroy callback, silently ignore
+		return 0
 	}
 	trailerMap := parseHeaderArray(trailers)
 	cb.OnResponseTrailersReceived(BidirectionalStream{stream}, trailerMap)
@@ -99,37 +95,33 @@ func bsOnResponseTrailersReceivedCallback(stream, trailers uintptr) uintptr {
 func bsOnSucceededCallback(stream uintptr) uintptr {
 	cb := instanceOfBidirectionalStreamCallback(stream)
 	if cb == nil {
-		return 0 // Post-destroy callback, silently ignore
+		return 0
 	}
 	cb.OnSucceeded(BidirectionalStream{stream})
-	// Terminal callback - safe to cleanup
 	cleanupBidirectionalStream(stream)
 	return 0
 }
 
 func bsOnFailedCallback(stream uintptr, netError int32) uintptr {
 	cb := instanceOfBidirectionalStreamCallback(stream)
 	if cb == nil {
-		return 0 // Post-destroy callback, silently ignore
+		return 0
 	}
 	cb.OnFailed(BidirectionalStream{stream}, int(netError))
-	// Terminal callback - safe to cleanup
 	cleanupBidirectionalStream(stream)
 	return 0
 }
 
 func bsOnCanceledCallback(stream uintptr) uintptr {
 	cb := instanceOfBidirectionalStreamCallback(stream)
 	if cb == nil {
-		return 0 // Post-destroy callback, silently ignore
+		return 0
 	}
 	cb.OnCanceled(BidirectionalStream{stream})
-	// Terminal callback - safe to cleanup
 	cleanupBidirectionalStream(stream)
 	return 0
 }
 
-// parseHeaderArray parses the bidirectional_stream_header_array pointer into a Go map
 func parseHeaderArray(ptr uintptr) map[string]string {
 	if ptr == 0 {
 		return nil

bidirectional_stream_map.go

@@ -5,9 +5,6 @@ import (
 	"sync/atomic"
 )
 
-// bidirectionalStreamEntry holds a callback and its destroyed state.
-// The destroyed flag is used to handle async destruction - callbacks
-// may be invoked after Destroy() returns due to Cronet's async API.
 type bidirectionalStreamEntry struct {
 	callback  BidirectionalStreamCallback
 	destroyed atomic.Bool
@@ -22,9 +19,6 @@ func init() {
 	bidirectionalStreamMap = make(map[uintptr]*bidirectionalStreamEntry)
 }
 
-// instanceOfBidirectionalStreamCallback returns the callback for the given stream pointer.
-// Returns nil if the stream was destroyed or not found. Callers should silently
-// return on nil as post-destroy callbacks are expected async API behavior.
 func instanceOfBidirectionalStreamCallback(ptr uintptr) BidirectionalStreamCallback {
 	bidirectionalStreamAccess.RLock()
 	defer bidirectionalStreamAccess.RUnlock()
@@ -35,8 +29,6 @@ func instanceOfBidirectionalStreamCallback(ptr uintptr) BidirectionalStreamCallb
 	return entry.callback
 }
 
-// cleanupBidirectionalStream removes the stream entry from the map.
-// Should be called from terminal callbacks (OnSucceeded/OnFailed/OnCanceled).
 func cleanupBidirectionalStream(ptr uintptr) {
 	bidirectionalStreamAccess.Lock()
 	delete(bidirectionalStreamMap, ptr)