feat: unify native API invocation with async callbacks (#1476)

bind/desktop/main.go

@@ -6,9 +6,15 @@ package main
 
 typedef void (*TaskEventCallback)(char* payload);
 
+typedef void (*InvokeResultCallback)(uint64_t request_id, int success, char* payload);
+
 static void callTaskEventCallback(uintptr_t callback, char* payload) {
 	((TaskEventCallback)callback)(payload);
 }
+
+static void callInvokeResultCallback(uintptr_t callback, uint64_t request_id, int success, char* payload) {
+	((InvokeResultCallback)callback)(request_id, success, payload);
+}
 */
 import "C"
 import (
@@ -16,6 +22,7 @@ import (
 	"fmt"
 	"unsafe"
 
+	nativebridge "github.com/GopeedLab/gopeed/bind/native"
 	"github.com/GopeedLab/gopeed/pkg/rest"
 	"github.com/GopeedLab/gopeed/pkg/rest/model"
 )
@@ -35,12 +42,13 @@ func Start(cfg *C.char) (int, *C.char) {
 	if err != nil {
 		return 0, C.CString(err.Error())
 	}
+	nativebridge.ResumeInvokes()
 	return realPort, nil
 }
 
 //export Stop
 func Stop() {
-	rest.Stop()
+	nativebridge.Stop()
 }
 
 //export GetAPIServerState
@@ -75,14 +83,39 @@ func apiServerResult(state *model.APIServerState, err error) string {
 	return string(data)
 }
 
-//export Invoke
-func Invoke(method *C.char, path *C.char, query *C.char, body *C.char) *C.char {
-	return C.CString(rest.Invoke(
+//export InvokeAsync
+func InvokeAsync(
+	method *C.char,
+	path *C.char,
+	query *C.char,
+	body *C.char,
+	requestID C.ulonglong,
+	callback C.uintptr_t,
+) {
+	if callback == 0 {
+		return
+	}
+	complete := func(result string, err error) {
+		success := C.int(1)
+		payload := result
+		if err != nil {
+			success = 0
+			payload = err.Error()
+		}
+		C.callInvokeResultCallback(
+			callback,
+			C.uint64_t(requestID),
+			success,
+			C.CString(payload),
+		)
+	}
+	nativebridge.InvokeAsync(
 		C.GoString(method),
 		C.GoString(path),
 		C.GoString(query),
 		C.GoString(body),
-	))
+		complete,
+	)
 }
 
 //export SubscribeTaskEvents

bind/mobile/main.go

@@ -5,6 +5,7 @@ import "C"
 import (
 	"encoding/json"
 
+	nativebridge "github.com/GopeedLab/gopeed/bind/native"
 	"github.com/GopeedLab/gopeed/internal/webview/rpcprovider"
 	"github.com/GopeedLab/gopeed/pkg/rest"
 	"github.com/GopeedLab/gopeed/pkg/rest/model"
@@ -18,11 +19,15 @@ func Start(cfg string) (int, error) {
 	config.ProductionMode = true
 	config.NativeMode = true
 	applyWebViewProvider(&config)
-	return rest.Start(&config)
+	port, err := rest.Start(&config)
+	if err == nil {
+		nativebridge.ResumeInvokes()
+	}
+	return port, err
 }
 
 func Stop() {
-	rest.Stop()
+	nativebridge.Stop()
 }
 
 func GetAPIServerState() string {
@@ -50,8 +55,21 @@ func apiServerResult(state *model.APIServerState, err error) string {
 	return string(data)
 }
 
-func Invoke(method, path, query, body string) string {
-	return rest.Invoke(method, path, query, body)
+type InvokeResultListener interface {
+	OnResult(requestID int64, success bool, payload string)
+}
+
+func InvokeAsync(method, path, query, body string, requestID int64, listener InvokeResultListener) {
+	if listener == nil {
+		return
+	}
+	nativebridge.InvokeAsync(method, path, query, body, func(result string, err error) {
+		if err != nil {
+			listener.OnResult(requestID, false, err.Error())
+			return
+		}
+		listener.OnResult(requestID, true, result)
+	})
 }
 
 type TaskEventListener interface {

bind/native/invoke.go

@@ -0,0 +1,240 @@
+// Package nativebridge contains transport-neutral behavior shared by the
+// Desktop C FFI and gomobile bridge adapters.
+package nativebridge
+
+import (
+	"errors"
+	"fmt"
+	"sync"
+	"time"
+
+	"github.com/GopeedLab/gopeed/pkg/rest"
+)
+
+// StopTimeout bounds native shutdown even when an in-process request never returns.
+const StopTimeout = 3 * time.Second
+
+var errNativeBridgeStopping = errors.New("native bridge is stopping")
+
+type invokeRequest struct {
+	method string
+	path   string
+	query  string
+	body   string
+}
+
+type invokeTask struct {
+	id       uint64
+	request  invokeRequest
+	complete func(result string, err error)
+	once     sync.Once
+}
+
+func (t *invokeTask) finish(result string, err error) {
+	t.once.Do(func() {
+		completeCallback(t.complete, result, err)
+	})
+}
+
+// InvokeAsync starts an in-process API request and completes it through the
+// supplied callback. Desktop and mobile adapters share this executor.
+func InvokeAsync(method, path, query, body string, callback func(result string, err error)) {
+	if callback == nil {
+		return
+	}
+	task := invokeTask{
+		request: invokeRequest{
+			method: method,
+			path:   path,
+			query:  query,
+			body:   body,
+		},
+		complete: callback,
+	}
+	if !getInvokeExecutor().submit(task) {
+		completeCallback(callback, "", errNativeBridgeStopping)
+	}
+}
+
+// ResumeInvokes enables request submission after the native runtime starts.
+func ResumeInvokes() {
+	getInvokeExecutor().resume()
+}
+
+// PauseInvokesAndWait prevents new submissions and gives running requests a
+// bounded opportunity to finish before a native runtime is stopped.
+// Requests still pending after StopTimeout are failed and detached so shutdown
+// can continue without invoking their callbacks a second time.
+func PauseInvokesAndWait() bool {
+	return getInvokeExecutor().pauseAndWait(StopTimeout)
+}
+
+// Stop pauses native requests and shuts down the shared runtime, but never
+// keeps the host application waiting longer than StopTimeout. Cleanup that
+// cannot finish within the deadline is allowed to continue in the background.
+func Stop() {
+	stopWithin(getInvokeExecutor(), rest.Stop, StopTimeout)
+}
+
+func stopWithin(executor *invokeExecutor, cleanup func(), timeout time.Duration) {
+	deadline := time.Now().Add(timeout)
+	executor.pauseAndWait(time.Until(deadline))
+
+	stopped := make(chan struct{})
+	go func() {
+		defer close(stopped)
+		cleanup()
+	}()
+
+	remaining := time.Until(deadline)
+	if remaining <= 0 {
+		return
+	}
+	timer := time.NewTimer(remaining)
+	defer timer.Stop()
+	select {
+	case <-stopped:
+	case <-timer.C:
+	}
+}
+
+var (
+	invokeExecutorOnce sync.Once
+	sharedInvoker      *invokeExecutor
+)
+
+func getInvokeExecutor() *invokeExecutor {
+	invokeExecutorOnce.Do(func() {
+		sharedInvoker = newInvokeExecutor(func(request invokeRequest) string {
+			return rest.Dispatch(request.method, request.path, request.query, request.body)
+		})
+	})
+	return sharedInvoker
+}
+
+// invokeExecutor starts every native API request in its own goroutine, matching
+// net/http's request concurrency. It only tracks them to support bounded drain
+// during shutdown; one stuck request must never prevent another from starting.
+type invokeExecutor struct {
+	mu        sync.Mutex
+	closed    bool
+	accepting bool
+	nextID    uint64
+	pending   map[uint64]*invokeTask
+	drained   chan struct{}
+	running   sync.WaitGroup
+	handler   func(invokeRequest) string
+}
+
+func newInvokeExecutor(handler func(invokeRequest) string) *invokeExecutor {
+	drained := make(chan struct{})
+	close(drained)
+	return &invokeExecutor{
+		handler:   handler,
+		accepting: true,
+		pending:   make(map[uint64]*invokeTask),
+		drained:   drained,
+	}
+}
+
+func (e *invokeExecutor) submit(task invokeTask) bool {
+	e.mu.Lock()
+	if e.closed || !e.accepting {
+		e.mu.Unlock()
+		return false
+	}
+	if len(e.pending) == 0 {
+		e.drained = make(chan struct{})
+	}
+	taskID := e.nextID
+	e.nextID++
+	task.id = taskID
+	taskPtr := &task
+	e.pending[taskID] = taskPtr
+	e.running.Add(1)
+	e.mu.Unlock()
+
+	go e.run(taskPtr)
+	return true
+}
+
+func (e *invokeExecutor) resume() {
+	e.mu.Lock()
+	e.accepting = true
+	e.mu.Unlock()
+}
+
+func (e *invokeExecutor) pauseAndWait(timeout time.Duration) bool {
+	e.mu.Lock()
+	e.accepting = false
+	if len(e.pending) == 0 {
+		e.mu.Unlock()
+		return true
+	}
+	drained := e.drained
+	e.mu.Unlock()
+
+	timer := time.NewTimer(timeout)
+	defer timer.Stop()
+	select {
+	case <-drained:
+		return true
+	case <-timer.C:
+	}
+
+	e.mu.Lock()
+	if len(e.pending) == 0 {
+		e.mu.Unlock()
+		return true
+	}
+	pending := make([]*invokeTask, 0, len(e.pending))
+	for _, task := range e.pending {
+		pending = append(pending, task)
+	}
+	e.pending = make(map[uint64]*invokeTask)
+	close(e.drained)
+	e.mu.Unlock()
+
+	for _, task := range pending {
+		task.finish("", errNativeBridgeStopping)
+	}
+	return false
+}
+
+func (e *invokeExecutor) close() {
+	e.mu.Lock()
+	e.closed = true
+	e.accepting = false
+	e.mu.Unlock()
+	e.running.Wait()
+}
+
+func (e *invokeExecutor) run(task *invokeTask) {
+	defer e.running.Done()
+	result, err := e.execute(task.request)
+	task.finish(result, err)
+	e.mu.Lock()
+	if _, exists := e.pending[task.id]; exists {
+		delete(e.pending, task.id)
+		if len(e.pending) == 0 {
+			close(e.drained)
+		}
+	}
+	e.mu.Unlock()
+}
+
+func (e *invokeExecutor) execute(request invokeRequest) (result string, err error) {
+	defer func() {
+		if recovered := recover(); recovered != nil {
+			err = fmt.Errorf("invoke panic: %v", recovered)
+		}
+	}()
+	return e.handler(request), nil
+}
+
+func completeCallback(callback func(string, error), result string, err error) {
+	defer func() {
+		_ = recover()
+	}()
+	callback(result, err)
+}

bind/native/invoke_test.go

@@ -0,0 +1,258 @@
+package nativebridge
+
+import (
+	"runtime"
+	"strconv"
+	"sync"
+	"testing"
+	"time"
+)
+
+func TestInvokeExecutorStartsEveryRequestIndependently(t *testing.T) {
+	const taskCount = 64
+	release := make(chan struct{})
+	started := make(chan struct{}, taskCount)
+	executor := newInvokeExecutor(func(request invokeRequest) string {
+		started <- struct{}{}
+		<-release
+		return request.path
+	})
+
+	var completed sync.WaitGroup
+	completed.Add(taskCount)
+	for i := range taskCount {
+		accepted := executor.submit(invokeTask{
+			request: invokeRequest{path: strconv.Itoa(i)},
+			complete: func(string, error) {
+				completed.Done()
+			},
+		})
+		if !accepted {
+			t.Fatalf("request %d was rejected", i)
+		}
+	}
+
+	// Every handler must start before any of them is released. A worker pool
+	// would stall here once all of its workers were occupied.
+	for range taskCount {
+		select {
+		case <-started:
+		case <-time.After(time.Second):
+			t.Fatal("a request waited behind other blocked requests")
+		}
+	}
+
+	close(release)
+	completed.Wait()
+	executor.close()
+}
+
+func TestInvokeExecutorConvertsPanicsToErrors(t *testing.T) {
+	executor := newInvokeExecutor(func(invokeRequest) string {
+		panic("boom")
+	})
+	completed := make(chan error, 1)
+	executor.submit(invokeTask{complete: func(_ string, err error) {
+		completed <- err
+	}})
+	select {
+	case err := <-completed:
+		if err == nil || err.Error() != "invoke panic: boom" {
+			t.Fatalf("unexpected error: %v", err)
+		}
+	case <-time.After(time.Second):
+		t.Fatal("panic result was not completed")
+	}
+	executor.close()
+}
+
+func TestInvokeExecutorSurvivesCallbackPanic(t *testing.T) {
+	executor := newInvokeExecutor(func(request invokeRequest) string {
+		return request.path
+	})
+	executor.submit(invokeTask{complete: func(string, error) {
+		panic("callback failed")
+	}})
+
+	completed := make(chan string, 1)
+	executor.submit(invokeTask{
+		request: invokeRequest{path: "next"},
+		complete: func(result string, _ error) {
+			completed <- result
+		},
+	})
+	select {
+	case result := <-completed:
+		if result != "next" {
+			t.Fatalf("unexpected result: %q", result)
+		}
+	case <-time.After(time.Second):
+		t.Fatal("worker stopped after callback panic")
+	}
+	executor.close()
+}
+
+func TestInvokeExecutorPausesAndDrains(t *testing.T) {
+	release := make(chan struct{})
+	started := make(chan struct{})
+	executor := newInvokeExecutor(func(request invokeRequest) string {
+		if request.path == "running" {
+			close(started)
+			<-release
+		}
+		return request.path
+	})
+	completed := make(chan struct{})
+	if !executor.submit(invokeTask{
+		request: invokeRequest{path: "running"},
+		complete: func(string, error) {
+			close(completed)
+		},
+	}) {
+		t.Fatal("initial task was rejected")
+	}
+	<-started
+
+	drained := make(chan struct{})
+	go func() {
+		executor.pauseAndWait(time.Second)
+		close(drained)
+	}()
+	deadline := time.Now().Add(time.Second)
+	for {
+		executor.mu.Lock()
+		paused := !executor.accepting
+		executor.mu.Unlock()
+		if paused {
+			break
+		}
+		if time.Now().After(deadline) {
+			t.Fatal("executor did not pause")
+		}
+		runtime.Gosched()
+	}
+	select {
+	case <-drained:
+		t.Fatal("pause returned before the running request completed")
+	default:
+	}
+	if executor.submit(invokeTask{}) {
+		t.Fatal("paused executor accepted a new request")
+	}
+
+	close(release)
+	<-completed
+	select {
+	case <-drained:
+	case <-time.After(time.Second):
+		t.Fatal("pause did not return after pending requests drained")
+	}
+
+	executor.resume()
+	resumed := make(chan struct{})
+	if !executor.submit(invokeTask{complete: func(string, error) {
+		close(resumed)
+	}}) {
+		t.Fatal("resumed executor rejected a request")
+	}
+	<-resumed
+	executor.close()
+}
+
+func TestInvokeExecutorPauseTimesOutAndFailsPendingRequests(t *testing.T) {
+	release := make(chan struct{})
+	started := make(chan struct{})
+	executor := newInvokeExecutor(func(request invokeRequest) string {
+		started <- struct{}{}
+		<-release
+		return request.path
+	})
+
+	errors := make(chan error, 2)
+	for _, path := range []string{"first", "second"} {
+		if !executor.submit(invokeTask{
+			request: invokeRequest{path: path},
+			complete: func(_ string, err error) {
+				errors <- err
+			},
+		}) {
+			t.Fatalf("request %q was rejected", path)
+		}
+	}
+	for range 2 {
+		<-started
+	}
+
+	const timeout = 30 * time.Millisecond
+	startedAt := time.Now()
+	if executor.pauseAndWait(timeout) {
+		t.Fatal("pause unexpectedly reported a complete drain")
+	}
+	if elapsed := time.Since(startedAt); elapsed < timeout || elapsed > 10*timeout {
+		t.Fatalf("pause returned after %s, want close to %s", elapsed, timeout)
+	}
+	for range 2 {
+		select {
+		case err := <-errors:
+			if err == nil || err.Error() != errNativeBridgeStopping.Error() {
+				t.Fatalf("unexpected timeout error: %v", err)
+			}
+		case <-time.After(time.Second):
+			t.Fatal("pending request was not failed after the timeout")
+		}
+	}
+
+	close(release)
+	executor.close()
+	select {
+	case err := <-errors:
+		t.Fatalf("request callback ran more than once: %v", err)
+	default:
+	}
+}
+
+func TestStopWithinUsesOneDeadlineForRequestsAndCleanup(t *testing.T) {
+	releaseRequest := make(chan struct{})
+	requestStarted := make(chan struct{})
+	executor := newInvokeExecutor(func(invokeRequest) string {
+		close(requestStarted)
+		<-releaseRequest
+		return "done"
+	})
+	requestResult := make(chan error, 1)
+	executor.submit(invokeTask{complete: func(_ string, err error) {
+		requestResult <- err
+	}})
+	<-requestStarted
+
+	releaseCleanup := make(chan struct{})
+	cleanupStarted := make(chan struct{})
+	cleanup := func() {
+		close(cleanupStarted)
+		<-releaseCleanup
+	}
+
+	const timeout = 40 * time.Millisecond
+	startedAt := time.Now()
+	stopWithin(executor, cleanup, timeout)
+	if elapsed := time.Since(startedAt); elapsed < timeout || elapsed > 10*timeout {
+		t.Fatalf("stop returned after %s, want close to %s", elapsed, timeout)
+	}
+	select {
+	case <-cleanupStarted:
+	case <-time.After(time.Second):
+		t.Fatal("cleanup was not started after the request deadline")
+	}
+	select {
+	case err := <-requestResult:
+		if err == nil || err.Error() != errNativeBridgeStopping.Error() {
+			t.Fatalf("unexpected request result: %v", err)
+		}
+	case <-time.After(time.Second):
+		t.Fatal("timed-out request was not failed")
+	}
+
+	close(releaseRequest)
+	close(releaseCleanup)
+	executor.close()
+}