[assetServer] Improve release/close handling of webview requests (#2612)

v2/internal/frontend/desktop/darwin/frontend.go

@@ -113,7 +113,6 @@ func (f *Frontend) startMessageProcessor() {
 func (f *Frontend) startRequestProcessor() {
 	for request := range requestBuffer {
 		f.assets.ServeWebViewRequest(request)
-		request.Release()
 	}
 }
 func (f *Frontend) startCallbackProcessor() {

v2/internal/frontend/desktop/linux/frontend.go

@@ -466,7 +466,6 @@ var requestBuffer = make(chan webview.Request, 100)
 func (f *Frontend) startRequestProcessor() {
 	for request := range requestBuffer {
 		f.assets.ServeWebViewRequest(request)
-		request.Release()
 	}
 }
 

v2/pkg/assetserver/assetserver_legacy.go

@@ -56,13 +56,7 @@ func (r legacyRequest) Response() webview.ResponseWriter {
 	return &legacyRequestNoOpCloserResponseWriter{r.rw}
 }
 
-func (r legacyRequest) AddRef() error {
+func (r legacyRequest) Close() error { return nil }
-	return nil
-}
-
-func (r legacyRequest) Release() error {
-	return nil
-}
 
 func (r *legacyRequest) request() (*http.Request, error) {
 	if r.req != nil {
@@ -81,6 +75,4 @@ type legacyRequestNoOpCloserResponseWriter struct {
 	http.ResponseWriter
 }
 
-func (*legacyRequestNoOpCloserResponseWriter) Finish() error {
+func (*legacyRequestNoOpCloserResponseWriter) Finish() {}
-	return nil
-}

v2/pkg/assetserver/assetserver_webview.go

@@ -22,6 +22,7 @@ type assetServerWebView struct {
 
 // ServeWebViewRequest processes the HTTP Request asynchronously by faking a golang HTTP Server.
 // The request will be finished with a StatusNotImplemented code if no handler has written to the response.
+// The AssetServer takes ownership of the request and the caller mustn't close it or access it in any other way.
 func (d *AssetServer) ServeWebViewRequest(req webview.Request) {
 	d.dispatchInit.Do(func() {
 		workers := d.dispatchWorkers
@@ -33,8 +34,11 @@ func (d *AssetServer) ServeWebViewRequest(req webview.Request) {
 		for i := 0; i < workers; i++ {
 			go func() {
 				for req := range workerC {
+					uri, _ := req.URL()
 					d.processWebViewRequest(req)
-					req.Release()
+					if err := req.Close(); err != nil {
+						d.logError("Unable to call close for request for uri '%s'", uri)
+					}
 				}
 			}()
 		}
@@ -45,12 +49,6 @@ func (d *AssetServer) ServeWebViewRequest(req webview.Request) {
 		d.dispatchReqC = dispatchC
 	})
 
-	if err := req.AddRef(); err != nil {
-		uri, _ := req.URL()
-		d.logError("Unable to call AddRef for request '%s'", uri)
-		return
-	}
-
 	d.dispatchReqC <- req
 }
 

v2/pkg/assetserver/webview/request.go

@@ -13,6 +13,5 @@ type Request interface {
 
 	Response() ResponseWriter
 
-	AddRef() error
+	Close() error
-	Release() error
 }

v2/pkg/assetserver/webview/request_darwin.go

@@ -118,11 +118,9 @@ import (
 )
 
 // NewRequest creates as new WebViewRequest based on a pointer to an `id<WKURLSchemeTask>`
-//
-// Please make sure to call Release() when finished using the request.
 func NewRequest(wkURLSchemeTask unsafe.Pointer) Request {
 	C.URLSchemeTaskRetain(wkURLSchemeTask)
-	return &request{task: wkURLSchemeTask}
+	return newRequestFinalizer(&request{task: wkURLSchemeTask})
 }
 
 var _ Request = &request{}
@@ -135,16 +133,6 @@ type request struct {
 	rw     *responseWriter
 }
 
-func (r *request) AddRef() error {
-	C.URLSchemeTaskRetain(r.task)
-	return nil
-}
-
-func (r *request) Release() error {
-	C.URLSchemeTaskRelease(r.task)
-	return nil
-}
-
 func (r *request) URL() (string, error) {
 	return C.GoString(C.URLSchemeTaskRequestURL(r.task)), nil
 }
@@ -205,6 +193,16 @@ func (r *request) Response() ResponseWriter {
 	return r.rw
 }
 
+func (r *request) Close() error {
+	var err error
+	if r.body != nil {
+		err = r.body.Close()
+	}
+	r.Response().Finish()
+	C.URLSchemeTaskRelease(r.task)
+	return err
+}
+
 var _ io.ReadCloser = &requestBodyStreamReader{}
 
 type requestBodyStreamReader struct {

v2/pkg/assetserver/webview/request_finalizer.go

@@ -0,0 +1,40 @@
+package webview
+
+import (
+	"runtime"
+	"sync/atomic"
+)
+
+var _ Request = &requestFinalizer{}
+
+type requestFinalizer struct {
+	Request
+	closed int32
+}
+
+// newRequestFinalizer returns a request with a runtime finalizer to make sure it will be closed from the finalizer
+// if it has not been already closed.
+// It also makes sure Close() of the wrapping request is only called once.
+func newRequestFinalizer(r Request) Request {
+	rf := &requestFinalizer{Request: r}
+	// Make sure to async release since it might block the finalizer goroutine for a longer period
+	runtime.SetFinalizer(rf, func(obj *requestFinalizer) { rf.close(true) })
+	return rf
+}
+
+func (r *requestFinalizer) Close() error {
+	return r.close(false)
+}
+
+func (r *requestFinalizer) close(asyncRelease bool) error {
+	if atomic.CompareAndSwapInt32(&r.closed, 0, 1) {
+		runtime.SetFinalizer(r, nil)
+		if asyncRelease {
+			go r.Request.Close()
+			return nil
+		} else {
+			return r.Request.Close()
+		}
+	}
+	return nil
+}

v2/pkg/assetserver/webview/request_linux.go

@@ -18,13 +18,12 @@ import (
 )
 
 // NewRequest creates as new WebViewRequest based on a pointer to an `WebKitURISchemeRequest`
-//
-// Please make sure to call Release() when finished using the request.
 func NewRequest(webKitURISchemeRequest unsafe.Pointer) Request {
 	webkitReq := (*C.WebKitURISchemeRequest)(webKitURISchemeRequest)
+	C.g_object_ref(C.gpointer(webkitReq))
+
 	req := &request{req: webkitReq}
-	req.AddRef()
+	return newRequestFinalizer(req)
-	return req
 }
 
 var _ Request = &request{}
@@ -37,16 +36,6 @@ type request struct {
 	rw     *responseWriter
 }
 
-func (r *request) AddRef() error {
-	C.g_object_ref(C.gpointer(r.req))
-	return nil
-}
-
-func (r *request) Release() error {
-	C.g_object_unref(C.gpointer(r.req))
-	return nil
-}
-
 func (r *request) URL() (string, error) {
 	return C.GoString(C.webkit_uri_scheme_request_get_uri(r.req)), nil
 }
@@ -82,3 +71,13 @@ func (r *request) Response() ResponseWriter {
 	r.rw = &responseWriter{req: r.req}
 	return r.rw
 }
+
+func (r *request) Close() error {
+	var err error
+	if r.body != nil {
+		err = r.body.Close()
+	}
+	r.Response().Finish()
+	C.g_object_unref(C.gpointer(r.req))
+	return err
+}

v2/pkg/assetserver/webview/responsewriter.go

@@ -20,6 +20,6 @@ var (
 type ResponseWriter interface {
 	http.ResponseWriter
 
-	// Finish the response and flush all data.
+	// Finish the response and flush all data. A Finish after the request has already been finished has no effect.
-	Finish() error
+	Finish()
 }

v2/pkg/assetserver/webview/responsewriter_darwin.go

@@ -133,16 +133,15 @@ func (rw *responseWriter) WriteHeader(code int) {
 	C.URLSchemeTaskDidReceiveResponse(rw.r.task, C.int(code), headers, C.int(headersLen))
 }
 
-func (rw *responseWriter) Finish() error {
+func (rw *responseWriter) Finish() {
 	if !rw.wroteHeader {
 		rw.WriteHeader(http.StatusNotImplemented)
 	}
 
 	if rw.finished {
-		return nil
+		return
 	}
 	rw.finished = true
 
 	C.URLSchemeTaskDidFinish(rw.r.task)
-	return nil
 }

v2/pkg/assetserver/webview/responsewriter_linux.go

@@ -84,19 +84,18 @@ func (rw *responseWriter) WriteHeader(code int) {
 	}
 }
 
-func (rw *responseWriter) Finish() error {
+func (rw *responseWriter) Finish() {
 	if !rw.wroteHeader {
 		rw.WriteHeader(http.StatusNotImplemented)
 	}
 
 	if rw.finished {
-		return nil
+		return
 	}
 	rw.finished = true
 	if rw.w != nil {
 		rw.w.Close()
 	}
-	return nil
 }
 
 func (rw *responseWriter) finishWithError(code int, err error) {

v3/pkg/application/application.go

@@ -324,10 +324,6 @@ func (a *App) Run() error {
 		for {
 			request := <-webviewRequests
 			a.handleWebViewRequest(request)
-			err := request.Release()
-			if err != nil {
-				a.error("Failed to release webview request: %s", err.Error())
-			}
 		}
 	}()
 	go func() {