net/http: release callbacks after fetch promise completes

When the request context was canceled, the Transport.RoundTrip method
could return before the fetch promise resolved. This would cause the
success and failure callback functions to get called after they've
been released, which in turn prints a "call to released function"
error to the console.

Avoid that problem by releasing the callbacks after the fetch promise
completes, by moving the release calls into the callbacks themselves.
This way we can still return from the Transport.RoundTrip method as
soon as the context is canceled, without waiting on the promise to
resolve. If the AbortController is unavailable and it's not possible to
abort the fetch operation, the promise may take a long time to resolve.

For #38003.

Change-Id: Ied1475e31dcba101b3326521b0cd653dbb345e1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/226204
Reviewed-by: Johan Brandhorst <johan.brandhorst@gmail.com>
Reviewed-by: Richard Musiol <neelance@gmail.com>

1 files changed, 17 insertions, 18 deletions

diff --git a/src/net/http/roundtrip_js.go b/src/net/http/roundtrip_js.go
index e14f3f7152..509d229aad 100644
--- a/src/net/http/roundtrip_js.go
+++ b/src/net/http/roundtrip_js.go
@@ -102,12 +102,17 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
 		js.CopyBytesToJS(buf, body)
 		opt.Set("body", buf)
 	}
-	respPromise := js.Global().Call("fetch", req.URL.String(), opt)
+
+	fetchPromise := js.Global().Call("fetch", req.URL.String(), opt)
 	var (
-		respCh = make(chan *Response, 1)
-		errCh  = make(chan error, 1)
+		respCh           = make(chan *Response, 1)
+		errCh            = make(chan error, 1)
+		success, failure js.Func
 	)
-	success := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
+	success = js.FuncOf(func(this js.Value, args []js.Value) interface{} {
+		success.Release()
+		failure.Release()
+
 		result := args[0]
 		header := Header{}
 		// https://developer.mozilla.org/en-US/docs/Web/API/Headers/entries
@@ -141,35 +146,29 @@ func (t *Transport) RoundTrip(req *Request) (*Response, error) {
 		}
 
 		code := result.Get("status").Int()
-		select {
-		case respCh <- &Response{
+		respCh <- &Response{
 			Status:        fmt.Sprintf("%d %s", code, StatusText(code)),
 			StatusCode:    code,
 			Header:        header,
 			ContentLength: contentLength,
 			Body:          body,
 			Request:       req,
-		}:
-		case <-req.Context().Done():
 		}
 
 		return nil
 	})
-	defer success.Release()
-	failure := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
-		err := fmt.Errorf("net/http: fetch() failed: %s", args[0].Get("message").String())
-		select {
-		case errCh <- err:
-		case <-req.Context().Done():
-		}
+	failure = js.FuncOf(func(this js.Value, args []js.Value) interface{} {
+		success.Release()
+		failure.Release()
+		errCh <- fmt.Errorf("net/http: fetch() failed: %s", args[0].Get("message").String())
 		return nil
 	})
-	defer failure.Release()
-	respPromise.Call("then", success, failure)
+
+	fetchPromise.Call("then", success, failure)
 	select {
 	case <-req.Context().Done():
 		if !ac.IsUndefined() {
-			// Abort the Fetch request
+			// Abort the Fetch request.
 			ac.Call("abort")
 		}
 		return nil, req.Context().Err()