From 873296c90fd0c091855d729f79c5309e975364e9 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Wed, 15 Jan 2025 16:07:08 -0500 Subject: [PATCH 01/23] feat(client): add callback/event mechanism between TypeScript and Go --- client/go/outline/event/event.go | 81 ++++++++++++ client/go/outline/event/event_test.go | 183 ++++++++++++++++++++++++++ 2 files changed, 264 insertions(+) create mode 100644 client/go/outline/event/event.go create mode 100644 client/go/outline/event/event_test.go diff --git a/client/go/outline/event/event.go b/client/go/outline/event/event.go new file mode 100644 index 00000000000..a1da3e7a595 --- /dev/null +++ b/client/go/outline/event/event.go @@ -0,0 +1,81 @@ +// Copyright 2025 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package event provides a thread-safe event system for Outline. +// It allows components to subscribe to events and be notified when they occur. +// +// This package is also designed for cross-language invocations between Go and TypeScript. +// All data structures related to an event should be designed to be compatible between both languages. +package event + +import "sync" + +// EventName is a type alias for string that represents the name of an event. +// Using a dedicated type improves type safety when working with event names. +type EventName string + +// Listener is the interface that must be implemented by the object that wants to subscribe to events. +type Listener interface { + // When an event is triggered, Handle is called with the event data as well as the optional parameter + // that was passed during [Subscribe]. + Handle(eventData, param string) +} + +// listenerInfo holds the listener callback and the optional parameter provided during [Subscribe]. +type listenerInfo struct { + cb Listener + param string +} + +var ( + mu sync.RWMutex // Protects the listeners map + listeners = make(map[EventName]listenerInfo) // A map containing all event listeners +) + +// Subscribe registers a [Listener] for a given [EventName]. +// +// This function overwrites any existing listeners for the specified [EventName]. +// +// The provided [Listener] will be called when the event is invoked, along with the event data and the supplied param. +func Subscribe(evt EventName, cb Listener, param string) { + if evt == "" || cb == nil { + return + } + mu.Lock() + defer mu.Unlock() + + listeners[evt] = listenerInfo{cb, param} +} + +// Unsubscribe removes the listener for the specified [EventName]. +// +// Calling this function is safe even if the event has not been registered. +func Unsubscribe(evt EventName) { + mu.Lock() + defer mu.Unlock() + + delete(listeners, evt) +} + +// Raise triggers the specified [EventName] with the given data. +// +// This function will do nothing if no listener is registered. +func Raise(evt EventName, data string) { + mu.RLock() + defer mu.RUnlock() + + if l, ok := listeners[evt]; ok { + l.cb.Handle(data, l.param) + } +} diff --git a/client/go/outline/event/event_test.go b/client/go/outline/event/event_test.go new file mode 100644 index 00000000000..1b99a422e4b --- /dev/null +++ b/client/go/outline/event/event_test.go @@ -0,0 +1,183 @@ +// Copyright 2025 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package event + +import ( + "fmt" + "sync" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_InvalidParams(t *testing.T) { + cntLst := len(listeners) + + l := &testListener{} + Subscribe("", l, "") + Raise("", "") + Subscribe("TestEvent", nil, "") + Unsubscribe("") + + require.Equal(t, cntLst, len(listeners)) +} + +func Test_UnsubscribeNonExistEvent(t *testing.T) { + Unsubscribe("") + Unsubscribe("NonExistEventName") +} + +func Test_InvokeNonExistEvent(t *testing.T) { + Raise("", "") + Raise("NonExistEventName", "Param") +} + +func TestNoSubscription(t *testing.T) { + evt := EventName("testNoSubEvent") + l := &testListener{} + + Raise(evt, "data") + l.requireEqual(t, 0, "", "") + + Subscribe(evt, l, "param") + Unsubscribe(evt) + Raise(evt, "data2") + l.requireEqual(t, 0, "", "") +} + +func TestSingleSubscription(t *testing.T) { + evt := EventName("testSingleSubEvent") + l := &testListener{} + Subscribe(evt, l, "mySingleSubParam") + + Raise(evt, "mySingleSubData") + l.requireEqual(t, 1, "mySingleSubData", "mySingleSubParam") + + Raise(evt, "mySingleSubData2") + l.requireEqual(t, 2, "mySingleSubData2", "mySingleSubParam") + + Unsubscribe(evt) + Raise(evt, "mySingleSubData3") + l.requireEqual(t, 2, "mySingleSubData2", "mySingleSubParam") + + Subscribe(evt, l, "") + Raise(evt, "mySingleSubData4") + l.requireEqual(t, 3, "mySingleSubData4", "") +} + +func TestOverwriteSubscription(t *testing.T) { + evt := EventName("testOverwriteSubEvent") + l1 := &testListener{} + l2 := &testListener{} + + Subscribe(evt, l1, "param1") + Raise(evt, "data1") + l1.requireEqual(t, 1, "data1", "param1") + l2.requireEqual(t, 0, "", "") + + Subscribe(evt, l2, "param2") + Raise(evt, "data2") + l1.requireEqual(t, 1, "data1", "param1") + l2.requireEqual(t, 1, "data2", "param2") +} + +func TestMultipleEvents(t *testing.T) { + evt1 := EventName("testMultiEvt1") + evt2 := EventName("testMultiEvt2") + l1 := &testListener{} + l2 := &testListener{} + + Subscribe(evt1, l1, "p1") + Subscribe(evt2, l2, "p2") + + Raise(evt1, "d1") + l1.requireEqual(t, 1, "d1", "p1") + l2.requireEqual(t, 0, "", "") + + Raise(evt2, "d2") + l1.requireEqual(t, 1, "d1", "p1") + l2.requireEqual(t, 1, "d2", "p2") + + Raise(evt1, "d3") + l1.requireEqual(t, 2, "d3", "p1") + l2.requireEqual(t, 1, "d2", "p2") + + Raise(evt2, "d4") + l1.requireEqual(t, 2, "d3", "p1") + l2.requireEqual(t, 2, "d4", "p2") +} + +func TestConcurrentEvents(t *testing.T) { + const numEvents = 50 + const invokesPerEvent = 50 + + var wg sync.WaitGroup + + // Subscribe to events concurrently + listeners := make([]*testListener, numEvents) + wg.Add(numEvents) + for i := 0; i < numEvents; i++ { + go func(i int) { + defer wg.Done() + listeners[i] = &testListener{} + evtName := EventName(fmt.Sprintf("testConcurrentEvent-%d", i)) + Subscribe(evtName, listeners[i], fmt.Sprintf("param-%d", i)) + }(i) + } + wg.Wait() + + // Invoke events concurrently + wg.Add(numEvents * invokesPerEvent) + for i := 0; i < numEvents; i++ { + for j := 0; j < invokesPerEvent; j++ { + go func(i, j int) { + defer wg.Done() + evtName := EventName(fmt.Sprintf("testConcurrentEvent-%d", i)) + Raise(evtName, fmt.Sprintf("data-%d-%d", i, j)) + }(i, j) + } + } + wg.Wait() + + // Verify results + for i := 0; i < numEvents; i++ { + require.Equal(t, int32(invokesPerEvent), listeners[i].cnt.Load()) + require.Regexp(t, fmt.Sprintf("data-%d-\\d", i), listeners[i].lastData.Load()) + require.Equal(t, fmt.Sprintf("param-%d", i), listeners[i].lastParam.Load()) + } +} + +type testListener struct { + cnt atomic.Int32 + lastData, lastParam atomic.Value +} + +func (l *testListener) Handle(eventData, param string) { + l.cnt.Add(1) + l.lastData.Store(eventData) + l.lastParam.Store(param) +} + +func (l *testListener) requireEqual(t *testing.T, cnt int32, data string, param string) { + require.Equal(t, cnt, l.cnt.Load()) + if cnt == 0 { + require.Nil(t, l.lastData.Load()) + require.Nil(t, l.lastParam.Load()) + } else { + require.Equal(t, data, l.lastData.Load()) + require.Equal(t, param, l.lastParam.Load()) + } +} From 12cb5f5505356b489fb4416e3c9d8430cf4e1c04 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Wed, 15 Jan 2025 16:30:06 -0500 Subject: [PATCH 02/23] Add corresponding cgo code to TS --- client/electron/go_plugin.ts | 143 ++++++++++++++++++++++++++++++++--- 1 file changed, 133 insertions(+), 10 deletions(-) diff --git a/client/electron/go_plugin.ts b/client/electron/go_plugin.ts index 76fe00c9afc..daaa80af0ea 100644 --- a/client/electron/go_plugin.ts +++ b/client/electron/go_plugin.ts @@ -18,8 +18,6 @@ import koffi from 'koffi'; import {pathToBackendLibrary} from './app_paths'; -let invokeMethodFunc: Function | undefined; - /** * Calls a Go function by invoking the `InvokeMethod` function in the native backend library. * @@ -36,7 +34,85 @@ export async function invokeMethod( method: string, input: string ): Promise { - if (!invokeMethodFunc) { + console.debug(`[Backend] - calling InvokeMethod "${method}" ...`); + const result = await ensureCgo().invokeMethod(method, input); + console.debug(`[Backend] - InvokeMethod "${method}" returned`, result); + if (result.ErrorJson) { + throw Error(result.ErrorJson); + } + return result.Output; +} + +/** + * Represents a callback function for an event. + * @param data The event data string passed from the source. + * @param param The param string provided during registration. + */ +export type CallbackFunction = (data: string, param: string) => void; + +/** + * Subscribes to an event from the Go backend. + * + * @param name The name of the event to subscribe to. + * @param callback The callback function to be called when the event is fired. + * @param param An optional parameter to be passed to the callback function. + * @returns A Promise that resolves when the subscription is successful. + * + * @remarks Subscribing to an event will replace any previously subscribed callback for that event. + */ +export async function subscribeEvent( + name: string, + callback: CallbackFunction, + param: string | null = null +): Promise { + console.debug(`[Backend] - calling SubscribeEvent "${name}" "${param}" ...`); + await ensureCgo().subscribeEvent( + name, + registerKoffiCallback(name, callback), + param + ); + console.debug(`[Backend] - SubscribeEvent "${name}" done`); +} + +/** + * Unsubscribes from an event from the Go backend. + * + * @param name The name of the event to unsubscribe from. + * @returns A Promise that resolves when the unsubscription is successful. + */ +export async function unsubscribeEvent(name: string): Promise { + console.debug(`[Backend] - calling UnsubscribeEvent "${name}" ...`); + await ensureCgo().unsubscribeEvent(name); + unregisterKoffiCallback(name); + console.debug(`[Backend] - UnsubscribeEvent "${name}" done`); +} + +/** Interface containing the exported native CGo functions. */ +interface CgoFunctions { + // InvokeMethodResult InvokeMethod(char* method, char* input); + invokeMethod: Function; + + // void (*ListenerFunc)(const char *data, const char *param); + listenerFuncPtr: koffi.IKoffiCType; + + // void SubscribeEvent(char* eventName, ListenerFunc callback, char* param); + subscribeEvent: Function; + + // void UnsubscribeEvent(char* eventName); + unsubscribeEvent: Function; +} + +/** Singleton of the loaded native CGo functions. */ +let cgo: CgoFunctions | undefined; + +/** + * Ensures that the CGo functions are loaded and initialized, returning the singleton instance. + * + * @returns The loaded CGo functions singleton. + */ +function ensureCgo(): CgoFunctions { + if (!cgo) { + console.debug('[Backend] - initializing cgo environment ...'); const backendLib = koffi.load(pathToBackendLibrary()); // Define C strings and setup auto release @@ -51,16 +127,63 @@ export async function invokeMethod( Output: cgoString, ErrorJson: cgoString, }); - invokeMethodFunc = promisify( + const invokeMethod = promisify( backendLib.func('InvokeMethod', invokeMethodResult, ['str', 'str']).async ); + + // Define SubscribeEvent/UnsubscribeEvent data structures and function + const listenerFuncPtr = koffi.pointer( + koffi.proto('ListenerFunc', 'void', [cgoString, cgoString]) + ); + const subscribeEvent = promisify( + backendLib.func('SubscribeEvent', 'void', ['str', listenerFuncPtr, 'str']) + .async + ); + const unsubscribeEvent = promisify( + backendLib.func('UnsubscribeEvent', 'void', ['str']).async + ); + + // Cache them so we don't have to reload these functions + cgo = {invokeMethod, listenerFuncPtr, subscribeEvent, unsubscribeEvent}; + console.debug('[Backend] - cgo environment initialized'); } + return cgo; +} - console.debug(`[Backend] - calling InvokeMethod "${method}" ...`); - const result = await invokeMethodFunc(method, input); - console.debug(`[Backend] - InvokeMethod "${method}" returned`, result); - if (result.ErrorJson) { - throw Error(result.ErrorJson); +//#region Koffi's internal registration management + +const koffiCallbacks = new Map(); + +/** + * Registers a persistent JS callback function with Koffi. + * This will replace any previously registered functions to align with `subscribeEvent`. + * + * @param eventName The name of the event. + * @param jsCallback The JavaScript callback function. + * @returns The registered Koffi callback. + * @see https://koffi.dev/callbacks#registered-callbacks + */ +function registerKoffiCallback( + eventName: string, + jsCallback: CallbackFunction +): koffi.IKoffiRegisteredCallback { + unregisterKoffiCallback(eventName); + const koffiCb = koffi.register(jsCallback, ensureCgo().listenerFuncPtr); + koffiCallbacks.set(eventName, koffiCb); + return koffiCb; +} + +/** + * Unregisters a Koffi callback for a specific event. + * + * @param eventName The name of the event. + */ +function unregisterKoffiCallback(eventName: string): void { + const cb = koffiCallbacks.get(eventName); + if (cb) { + koffi.unregister(cb); + koffiCallbacks.delete(eventName); } - return result.Output; } + +//#endregion Koffi's internal registration management From af81acc7bf9eb10a4b15e9795853d9a292328331 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Wed, 15 Jan 2025 22:02:15 -0500 Subject: [PATCH 03/23] Expose C functions to subscribe/unsubcribe events --- client/go/outline/electron/go_plugin.go | 51 +++++++++++++++++++++++++ client/go/outline/event/event.go | 11 +++++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/client/go/outline/electron/go_plugin.go b/client/go/outline/electron/go_plugin.go index ea66d07d878..0b4317afd0a 100644 --- a/client/go/outline/electron/go_plugin.go +++ b/client/go/outline/electron/go_plugin.go @@ -29,6 +29,26 @@ typedef struct InvokeMethodResult_t // This error can be parsed by the PlatformError in TypeScript. const char *ErrorJson; } InvokeMethodResult; + +// ListenerFunc is a C function pointer type that represents a callback function. +// This callback function will be invoked when an event is emitted. +// +// - data: The event data, passed as a C string. +// - param: An optional parameter that was passed during [SubscribeEvent], also passed as a C string. +typedef void (*ListenerFunc)(const char *data, const char *param); + +// InvokeListenerFunc takes a ListenerFunc callback, event data, and a parameter, and invokes the +// callback with these arguments. +// +// This function is the glue code needed for Go to call C function pointers. +// +// - f: The C function pointer to be invoked. +// - data: The event data, passed as a C string. +// - param: An optional parameter, passed as a C string. +static void InvokeListenerFunc(ListenerFunc f, const char *data, const char *param) +{ + f(data, param); +} */ import "C" import ( @@ -38,6 +58,7 @@ import ( "unsafe" "github.com/Jigsaw-Code/outline-apps/client/go/outline" + "github.com/Jigsaw-Code/outline-apps/client/go/outline/event" "github.com/Jigsaw-Code/outline-apps/client/go/outline/platerrors" ) @@ -57,6 +78,36 @@ func InvokeMethod(method *C.char, input *C.char) C.InvokeMethodResult { } } +// cgoListener implements [event.Listener] and calls a C function pointer when an event is emitted. +type cgoListener struct { + cb C.ListenerFunc +} + +var _ event.Listener = (*cgoListener)(nil) + +// Handle forwards the event data and the parameter to the C callback function pointer. +func (l *cgoListener) Handle(eventData, param string) { + C.InvokeListenerFunc(l.cb, newCGoString(eventData), newCGoString(param)) +} + +// SubscribeEvent allows TypeScript to subscribe to events implemented by the event package. +// +// For more details, refer to the documentation of the [event.Subscribe]. +// +//export SubscribeEvent +func SubscribeEvent(eventName *C.char, callback C.ListenerFunc, param *C.char) { + event.Subscribe(event.EventName(C.GoString(eventName)), &cgoListener{callback}, C.GoString(param)) +} + +// UnsubscribeEvent allows TypeScript to unsubscribe from events. +// +// For more details, refer to the documentation of the [event.Unsubscribe]. +// +//export UnsubscribeEvent +func UnsubscribeEvent(eventName *C.char) { + event.Unsubscribe(event.EventName(C.GoString(eventName))) +} + // newCGoString allocates memory for a C string based on the given Go string. // It should be paired with [FreeCGoString] to avoid memory leaks. func newCGoString(s string) *C.char { diff --git a/client/go/outline/event/event.go b/client/go/outline/event/event.go index a1da3e7a595..e6dcc4f7332 100644 --- a/client/go/outline/event/event.go +++ b/client/go/outline/event/event.go @@ -19,7 +19,10 @@ // All data structures related to an event should be designed to be compatible between both languages. package event -import "sync" +import ( + "log/slog" + "sync" +) // EventName is a type alias for string that represents the name of an event. // Using a dedicated type improves type safety when working with event names. @@ -50,12 +53,14 @@ var ( // The provided [Listener] will be called when the event is invoked, along with the event data and the supplied param. func Subscribe(evt EventName, cb Listener, param string) { if evt == "" || cb == nil { + slog.Warn("empty event or listener is ignored") return } mu.Lock() defer mu.Unlock() listeners[evt] = listenerInfo{cb, param} + slog.Debug("successfully subscribed to event", "event", evt, "param", param) } // Unsubscribe removes the listener for the specified [EventName]. @@ -66,6 +71,7 @@ func Unsubscribe(evt EventName) { defer mu.Unlock() delete(listeners, evt) + slog.Debug("successfully ubsubscribed from event", "event", evt) } // Raise triggers the specified [EventName] with the given data. @@ -76,6 +82,9 @@ func Raise(evt EventName, data string) { defer mu.RUnlock() if l, ok := listeners[evt]; ok { + slog.Debug("firing event", "event", evt, "data", data, "param", l.param) l.cb.Handle(data, l.param) + } else { + slog.Debug("event fired but no handlers are found", "event", evt, "data", data) } } From 15e173570265c7f6c0283456bac43cb4fec32616 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Wed, 15 Jan 2025 22:17:42 -0500 Subject: [PATCH 04/23] raise and subscribe to VPN connection status change event --- client/electron/vpn_service.ts | 86 ++++++++++++++++++++++++++++++---- client/go/outline/vpn/vpn.go | 43 +++++++++++++++-- 2 files changed, 116 insertions(+), 13 deletions(-) diff --git a/client/electron/vpn_service.ts b/client/electron/vpn_service.ts index 331034705d4..ba8f3fe946a 100644 --- a/client/electron/vpn_service.ts +++ b/client/electron/vpn_service.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {invokeMethod} from './go_plugin'; +import {invokeMethod, subscribeEvent} from './go_plugin'; import { StartRequestJson, TunnelStatus, @@ -35,15 +35,12 @@ interface EstablishVpnRequest { transport: string; } -let currentRequestId: string | undefined = undefined; - export async function establishVpn(request: StartRequestJson) { - currentRequestId = request.id; - statusCb?.(currentRequestId, TunnelStatus.RECONNECTING); + subscribeVPNConnEvents(); const config: EstablishVpnRequest = { vpn: { - id: currentRequestId, + id: request.id, // TUN device name, being compatible with old code: // https://github.com/Jigsaw-Code/outline-apps/blob/client/linux/v1.14.0/client/electron/linux_proxy_controller/outline_proxy_controller.h#L203 @@ -72,13 +69,10 @@ export async function establishVpn(request: StartRequestJson) { }; await invokeMethod('EstablishVPN', JSON.stringify(config)); - statusCb?.(currentRequestId, TunnelStatus.CONNECTED); } export async function closeVpn(): Promise { - statusCb?.(currentRequestId!, TunnelStatus.DISCONNECTING); await invokeMethod('CloseVPN', ''); - statusCb?.(currentRequestId!, TunnelStatus.DISCONNECTED); } export type VpnStatusCallback = (id: string, status: TunnelStatus) => void; @@ -88,3 +82,77 @@ let statusCb: VpnStatusCallback | undefined = undefined; export function onVpnStatusChanged(cb: VpnStatusCallback): void { statusCb = cb; } + +/** + * Forwards the VPN connection status change event to the VpnStatusCallback. + * @param connJson The JSON string representing the VPNConn interface. + */ +function handleVpnConnectionStatusChanged(connJson: string) { + const conn = parseVPNConnJSON(connJson); + console.debug(`received ${StatusChangedEvent}`, conn); + switch (conn?.status) { + case VPNConnConnected: + statusCb?.(conn.id, TunnelStatus.CONNECTED); + break; + case VPNConnConnecting: + statusCb?.(conn.id, TunnelStatus.RECONNECTING); + break; + case VPNConnDisconnecting: + statusCb?.(conn.id, TunnelStatus.DISCONNECTING); + break; + case VPNConnDisconnected: + statusCb?.(conn.id, TunnelStatus.DISCONNECTED); + break; + } +} + +let vpnEventsSubscribed = false; + +/** + * Subscribes to all VPN connection related events. + * This function ensures that the subscription only happens once. + */ +function subscribeVPNConnEvents(): void { + if (!vpnEventsSubscribed) { + subscribeEvent(StatusChangedEvent, handleVpnConnectionStatusChanged); + vpnEventsSubscribed = true; + } +} + +//#region type definitions of VPNConnection in Go + +// The following constants and types should be aligned with the corresponding definitions +// in `./client/go/outline/vpn/vpn.go`. + +const StatusChangedEvent = 'VPNConnStatusChanged'; + +type VPNConnStatus = string; +const VPNConnConnecting: VPNConnStatus = 'Connecting'; +const VPNConnConnected: VPNConnStatus = 'Connected'; +const VPNConnDisconnecting: VPNConnStatus = 'Disconnecting'; +const VPNConnDisconnected: VPNConnStatus = 'Disconnected'; + +interface VPNConn { + readonly id: string; + readonly status: VPNConnStatus; +} + +function parseVPNConnJSON(json: string): VPNConn | null { + try { + const rawConn = JSON.parse(json); + if (!('id' in rawConn) || !rawConn.id || !('status' in rawConn)) { + return null; + } + if (!('status' in rawConn) || !rawConn.status) { + return null; + } + return { + id: rawConn.id, + status: rawConn.status, + }; + } catch { + return null; + } +} + +//#endregion type definitions of VPNConnection in Go diff --git a/client/go/outline/vpn/vpn.go b/client/go/outline/vpn/vpn.go index acf35e99cae..832d118a1bc 100644 --- a/client/go/outline/vpn/vpn.go +++ b/client/go/outline/vpn/vpn.go @@ -16,10 +16,12 @@ package vpn import ( "context" + "encoding/json" "io" "log/slog" "sync" + "github.com/Jigsaw-Code/outline-apps/client/go/outline/event" "github.com/Jigsaw-Code/outline-sdk/transport" ) @@ -47,9 +49,23 @@ type platformVPNConn interface { Close() error } +// ConnectionStatus represents the status of a [VPNConnection]. +type ConnectionStatus string + +const ( + ConnectionConnected ConnectionStatus = "Connected" + ConnectionDisconnected ConnectionStatus = "Disconnected" + ConnectionConnecting ConnectionStatus = "Connecting" + ConnectionDisconnecting ConnectionStatus = "Disconnecting" +) + +// ConnectionStatusChanged event will be raised when the status of a [VPNConnection] changes. +const ConnectionStatusChanged event.EventName = "VPNConnStatusChanged" + // VPNConnection represents a system-wide VPN connection. type VPNConnection struct { - ID string + ID string `json:"id"` + Status ConnectionStatus `json:"status"` cancelEst context.CancelFunc wgEst, wgCopy sync.WaitGroup @@ -63,6 +79,16 @@ type VPNConnection struct { var mu sync.Mutex var conn *VPNConnection +// SetStatus sets the [VPNConnection] Status and raises the [ConnectionStatusChanged] event. +func (c *VPNConnection) SetStatus(status ConnectionStatus) { + c.Status = status + if connJson, err := json.Marshal(c); err == nil { + event.Raise(ConnectionStatusChanged, string(connJson)) + } else { + slog.Warn("failed to marshal VPN connection", "err", err) + } +} + // EstablishVPN establishes a new active [VPNConnection] connecting to a [ProxyDevice] // with the given VPN [Config]. // It first closes any active [VPNConnection] using [CloseVPN], and then marks the @@ -81,7 +107,7 @@ func EstablishVPN( panic("a PacketListener must be provided") } - c := &VPNConnection{ID: conf.ID} + c := &VPNConnection{ID: conf.ID, Status: ConnectionDisconnected} ctx, c.cancelEst = context.WithCancel(ctx) if c.platform, err = newPlatformVPNConn(conf); err != nil { @@ -97,6 +123,14 @@ func EstablishVPN( } slog.Debug("establishing vpn connection ...", "id", c.ID) + c.SetStatus(ConnectionConnecting) + defer func() { + if err == nil { + c.SetStatus(ConnectionConnected) + } else { + c.SetStatus(ConnectionDisconnected) + } + }() if c.proxy, err = ConnectRemoteDevice(ctx, sd, pl); err != nil { slog.Error("failed to connect to the remote device", "err", err) @@ -154,15 +188,16 @@ func closeVPNNoLock() (err error) { return nil } + slog.Debug("terminating the global vpn connection...", "id", conn.ID) + conn.SetStatus(ConnectionDisconnecting) defer func() { if err == nil { slog.Info("vpn connection terminated", "id", conn.ID) + conn.SetStatus(ConnectionDisconnected) conn = nil } }() - slog.Debug("terminating the global vpn connection...", "id", conn.ID) - // Cancel the Establish process and wait conn.cancelEst() conn.wgEst.Wait() From 4a2c58d0b07f5e02f68ab9b8e1699be6eab5fb56 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Wed, 15 Jan 2025 22:25:15 -0500 Subject: [PATCH 05/23] Simplify parseVPNConnJSON --- client/electron/vpn_service.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/client/electron/vpn_service.ts b/client/electron/vpn_service.ts index ba8f3fe946a..f3e31058825 100644 --- a/client/electron/vpn_service.ts +++ b/client/electron/vpn_service.ts @@ -143,9 +143,6 @@ function parseVPNConnJSON(json: string): VPNConn | null { if (!('id' in rawConn) || !rawConn.id || !('status' in rawConn)) { return null; } - if (!('status' in rawConn) || !rawConn.status) { - return null; - } return { id: rawConn.id, status: rawConn.status, From 3ced93ea2564205917b1e59ab3d14ac9e10d4807 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Fri, 17 Jan 2025 20:56:13 -0500 Subject: [PATCH 06/23] decouple callback from event --- client/go/outline/callback/callback.go | 97 +++++++++ client/go/outline/callback/callback_test.go | 183 +++++++++++++++++ client/go/outline/event/event.go | 66 +++--- client/go/outline/event/event_test.go | 217 ++++++++++++-------- client/go/outline/vpn/vpn.go | 2 +- 5 files changed, 439 insertions(+), 126 deletions(-) create mode 100644 client/go/outline/callback/callback.go create mode 100644 client/go/outline/callback/callback_test.go diff --git a/client/go/outline/callback/callback.go b/client/go/outline/callback/callback.go new file mode 100644 index 00000000000..6d2975b5b15 --- /dev/null +++ b/client/go/outline/callback/callback.go @@ -0,0 +1,97 @@ +// Copyright 2025 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package callback provides a thread-safe mechanism for managing and invoking callbacks. +package callback + +import ( + "fmt" + "log/slog" + "sync" +) + +// Token can be used to uniquely identify a registered callback. +type Token string + +// Callback is an interface that can be implemented to receive callbacks. +type Callback interface { + OnCall(data string) +} + +var ( + mu sync.RWMutex + callbacks = make(map[uint32]Callback) + nextCbID uint32 = 1 +) + +// New registers a new callback and returns a unique callback token. +func New(c Callback) Token { + mu.Lock() + defer mu.Unlock() + + id := nextCbID + nextCbID++ + callbacks[id] = c + slog.Debug("callback created", "id", id) + return getTokenByID(id) +} + +// Delete removes a callback identified by the token. +// +// Calling this function is safe even if the callback has not been registered. +func Delete(token Token) { + mu.Lock() + defer mu.Unlock() + + if id, err := getIDByToken(token); err == nil { + delete(callbacks, id) + slog.Debug("callback deleted", "id", id) + } else { + slog.Warn("invalid callback token", "err", err, "token", token) + } +} + +// Call executes a callback identified by the token. +// +// Calling this function is safe even if the callback has not been registered. +func Call(token Token, data string) { + id, err := getIDByToken(token) + if err != nil { + slog.Warn("invalid callback token", "err", err, "token", token) + return + } + + mu.RLock() + cb, ok := callbacks[id] + mu.RUnlock() + + if !ok { + slog.Warn("callback not yet created", "id", id, "token", token) + return + } + slog.Debug("invoking callback", "id", id, "data", data) + cb.OnCall(data) +} + +// getTokenByID creates a string-based callback token from a number-based internal ID. +func getTokenByID(id uint32) Token { + return Token(fmt.Sprintf("cbid-%d", id)) +} + +// getIDByToken parses a number-based internal ID from a string-based callback token. +func getIDByToken(token Token) (uint32, error) { + var id uint32 + _, err := fmt.Sscanf(string(token), "cbid-%d", &id) + return id, err +} diff --git a/client/go/outline/callback/callback_test.go b/client/go/outline/callback/callback_test.go new file mode 100644 index 00000000000..81b371fbfd7 --- /dev/null +++ b/client/go/outline/callback/callback_test.go @@ -0,0 +1,183 @@ +// Copyright 2025 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package callback + +import ( + "fmt" + "sync" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_New(t *testing.T) { + curID := nextCbID + token := New(&testCallback{}) + require.Equal(t, curID+1, nextCbID) + require.Contains(t, callbacks, curID) + + require.NotEmpty(t, token) + require.Equal(t, fmt.Sprintf("cbid-%d", curID), string(token)) + + id, err := getIDByToken(token) + require.NoError(t, err) + require.Contains(t, callbacks, id) + require.Equal(t, id, curID) +} + +func Test_Delete(t *testing.T) { + curID := nextCbID + token := New(&testCallback{}) + require.Contains(t, callbacks, curID) + + Delete(token) + require.NotContains(t, callbacks, curID) + require.Equal(t, curID+1, nextCbID) + + Delete("invalid-token") + require.NotContains(t, callbacks, curID) + require.Equal(t, curID+1, nextCbID) + + Delete("cbid-99999999") + require.NotContains(t, callbacks, curID) + require.Equal(t, curID+1, nextCbID) +} + +func Test_Call(t *testing.T) { + c := &testCallback{} + token := New(c) + c.requireEqual(t, 0, "") + + Call(token, "arg1") + c.requireEqual(t, 1, "arg1") + + Call("invalid-token", "arg1") + c.requireEqual(t, 1, "arg1") // No change + + Call(token, "arg2") + c.requireEqual(t, 2, "arg2") + + Call("cbid-99999999", "arg3") + c.requireEqual(t, 2, "arg2") // No change +} + +func Test_ConcurrentCreate(t *testing.T) { + const numTokens = 1000 + + curID := nextCbID + originalLen := len(callbacks) + var wg sync.WaitGroup + + tokens := make([]Token, numTokens) + wg.Add(numTokens) + for i := 0; i < numTokens; i++ { + go func(i int) { + defer wg.Done() + tokens[i] = New(&testCallback{}) + require.NotEmpty(t, tokens[i]) + require.Regexp(t, `^cbid-\d+$`, tokens[i]) + }(i) + } + wg.Wait() + + require.Len(t, callbacks, originalLen+numTokens) + require.Equal(t, curID+numTokens, nextCbID) + tokenSet := make(map[Token]bool) + for _, token := range tokens { + require.False(t, tokenSet[token], "Duplicate token found: %s", token) + tokenSet[token] = true + + id, err := getIDByToken(token) + require.NoError(t, err) + require.Contains(t, callbacks, id) + } +} + +func Test_ConcurrentCall(t *testing.T) { + const numInvocations = 1000 + + curID := nextCbID + originalLen := len(callbacks) + + c := &testCallback{} + token := New(c) + + var wg sync.WaitGroup + wg.Add(numInvocations) + for i := 0; i < numInvocations; i++ { + go func(i int) { + defer wg.Done() + Call(token, fmt.Sprintf("data-%d", i)) + }(i) + } + wg.Wait() + + require.Equal(t, int32(numInvocations), c.cnt.Load()) + require.Regexp(t, `^data-\d+$`, c.lastData.Load()) + + require.Len(t, callbacks, originalLen+1) + require.Equal(t, curID+1, nextCbID) +} + +func Test_ConcurrentDelete(t *testing.T) { + const ( + numTokens = 50 + numDeletes = 1000 + ) + + curID := nextCbID + originalLen := len(callbacks) + + tokens := make([]Token, numTokens) + for i := 0; i < numTokens; i++ { + tokens[i] = New(&testCallback{}) + } + require.Len(t, callbacks, originalLen+numTokens) + require.Equal(t, curID+numTokens, nextCbID) + + var wg sync.WaitGroup + wg.Add(numDeletes) + for i := 0; i < numDeletes; i++ { + go func(i int) { + defer wg.Done() + Delete(tokens[i%numTokens]) + }(i) + } + wg.Wait() + + require.Len(t, callbacks, originalLen) + require.Equal(t, curID+numTokens, nextCbID) +} + +// testCallback is a mock implementation of callback.Callback for testing. +type testCallback struct { + cnt atomic.Int32 + lastData atomic.Value +} + +func (tc *testCallback) OnCall(data string) { + tc.cnt.Add(1) + tc.lastData.Store(data) +} + +func (tc *testCallback) requireEqual(t *testing.T, cnt int32, data string) { + require.Equal(t, cnt, tc.cnt.Load()) + if cnt == 0 { + require.Nil(t, tc.lastData.Load()) + } else { + require.Equal(t, data, tc.lastData.Load()) + } +} diff --git a/client/go/outline/event/event.go b/client/go/outline/event/event.go index e6dcc4f7332..c29c2dcf830 100644 --- a/client/go/outline/event/event.go +++ b/client/go/outline/event/event.go @@ -12,79 +12,63 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package event provides a thread-safe event system for Outline. -// It allows components to subscribe to events and be notified when they occur. -// -// This package is also designed for cross-language invocations between Go and TypeScript. -// All data structures related to an event should be designed to be compatible between both languages. +// Package event provides a thread-safe mechanism for managing and triggering events. package event import ( "log/slog" + "slices" "sync" + + "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" ) // EventName is a type alias for string that represents the name of an event. // Using a dedicated type improves type safety when working with event names. type EventName string -// Listener is the interface that must be implemented by the object that wants to subscribe to events. -type Listener interface { - // When an event is triggered, Handle is called with the event data as well as the optional parameter - // that was passed during [Subscribe]. - Handle(eventData, param string) -} - -// listenerInfo holds the listener callback and the optional parameter provided during [Subscribe]. -type listenerInfo struct { - cb Listener - param string -} - var ( - mu sync.RWMutex // Protects the listeners map - listeners = make(map[EventName]listenerInfo) // A map containing all event listeners + mu sync.RWMutex + listeners = make(map[EventName][]callback.Token) ) -// Subscribe registers a [Listener] for a given [EventName]. -// -// This function overwrites any existing listeners for the specified [EventName]. +// AddListener adds a [callback.Callback] to a given [EventName]. // -// The provided [Listener] will be called when the event is invoked, along with the event data and the supplied param. -func Subscribe(evt EventName, cb Listener, param string) { - if evt == "" || cb == nil { - slog.Warn("empty event or listener is ignored") +// The provided callback will be called when the event is invoked, along with the event data. +func AddListener(evt EventName, cb callback.Token) { + if evt == "" || cb == "" { + slog.Warn("empty event or callback are ignored") return } mu.Lock() defer mu.Unlock() - listeners[evt] = listenerInfo{cb, param} - slog.Debug("successfully subscribed to event", "event", evt, "param", param) + listeners[evt] = append(listeners[evt], cb) + slog.Debug("successfully subscribed to event", "event", evt, "callback", cb) } -// Unsubscribe removes the listener for the specified [EventName]. +// RemoveListener removes a [callback.Callback] from the specified [EventName]. // // Calling this function is safe even if the event has not been registered. -func Unsubscribe(evt EventName) { +func RemoveListener(evt EventName, cb callback.Token) { mu.Lock() defer mu.Unlock() - delete(listeners, evt) - slog.Debug("successfully ubsubscribed from event", "event", evt) + if cbs, ok := listeners[evt]; ok { + listeners[evt] = slices.DeleteFunc(cbs, func(t callback.Token) bool { return t == cb }) + } + slog.Debug("successfully ubsubscribed from event", "event", evt, "callback", cb) } -// Raise triggers the specified [EventName] with the given data. +// Fire triggers the specified [EventName], invoking all associated callbacks with the given data. // -// This function will do nothing if no listener is registered. -func Raise(evt EventName, data string) { +// Calling this function is safe even if the event has not been registered. +func Fire(evt EventName, data string) { mu.RLock() defer mu.RUnlock() - if l, ok := listeners[evt]; ok { - slog.Debug("firing event", "event", evt, "data", data, "param", l.param) - l.cb.Handle(data, l.param) - } else { - slog.Debug("event fired but no handlers are found", "event", evt, "data", data) + slog.Debug("firing event", "event", evt, "data", data) + for _, cb := range listeners[evt] { + callback.Call(cb, data) } } diff --git a/client/go/outline/event/event_test.go b/client/go/outline/event/event_test.go index 1b99a422e4b..97fb0471fcf 100644 --- a/client/go/outline/event/event_test.go +++ b/client/go/outline/event/event_test.go @@ -20,78 +20,121 @@ import ( "sync/atomic" "testing" + "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" "github.com/stretchr/testify/require" ) -func Test_InvalidParams(t *testing.T) { - cntLst := len(listeners) +func Test_AddListener(t *testing.T) { + originalCnt := len(listeners) + evt := EventName("TestAddListenerEvent") + cb := callback.New(&testListener{}) + AddListener(evt, cb) - l := &testListener{} - Subscribe("", l, "") - Raise("", "") - Subscribe("TestEvent", nil, "") - Unsubscribe("") - - require.Equal(t, cntLst, len(listeners)) + require.Len(t, listeners, originalCnt+1) + require.Len(t, listeners[evt], 1) + require.Equal(t, listeners[evt][0], cb) } -func Test_UnsubscribeNonExistEvent(t *testing.T) { - Unsubscribe("") - Unsubscribe("NonExistEventName") +func Test_RemoveListener(t *testing.T) { + originalCnt := len(listeners) + evt := EventName("TestRemoveListenerEvent") + cb1 := callback.New(&testListener{}) + cb2 := callback.New(&testListener{}) + + AddListener(evt, cb1) + AddListener(evt, cb2) + require.Len(t, listeners, originalCnt+1) + require.Len(t, listeners[evt], 2) + require.Equal(t, listeners[evt][0], cb1) + require.Equal(t, listeners[evt][1], cb2) + + RemoveListener(evt, cb1) + require.Len(t, listeners, originalCnt+1) + require.Len(t, listeners[evt], 1) + require.Equal(t, listeners[evt][0], cb2) } -func Test_InvokeNonExistEvent(t *testing.T) { - Raise("", "") - Raise("NonExistEventName", "Param") +func Test_ZeroParams(t *testing.T) { + originalCnt := len(listeners) + + l := &testListener{} + AddListener("", callback.New(l)) + Fire("", "") + AddListener("TestEvent_InvalidParam", "") + + RemoveListener("", callback.New(l)) + RemoveListener("TestEvent_InvalidParam", "") + RemoveListener("NonExistEventName", callback.New(&testListener{})) + + Fire("", "data1") + Fire("NonExistEventName", "data2") + + require.Len(t, listeners, originalCnt) } -func TestNoSubscription(t *testing.T) { +func Test_NoSubscription(t *testing.T) { evt := EventName("testNoSubEvent") l := &testListener{} + cb := callback.New(l) - Raise(evt, "data") - l.requireEqual(t, 0, "", "") + Fire(evt, "data") + l.requireEqual(t, 0, "") // No listener, callback should not be called - Subscribe(evt, l, "param") - Unsubscribe(evt) - Raise(evt, "data2") - l.requireEqual(t, 0, "", "") + AddListener(evt, cb) + RemoveListener(evt, cb) + Fire(evt, "data2") + l.requireEqual(t, 0, "") // Listener removed, callback should not be called } -func TestSingleSubscription(t *testing.T) { +func Test_SingleSubscription(t *testing.T) { evt := EventName("testSingleSubEvent") l := &testListener{} - Subscribe(evt, l, "mySingleSubParam") + cb := callback.New(l) + AddListener(evt, cb) - Raise(evt, "mySingleSubData") - l.requireEqual(t, 1, "mySingleSubData", "mySingleSubParam") + Fire(evt, "mySingleSubData") + l.requireEqual(t, 1, "mySingleSubData") - Raise(evt, "mySingleSubData2") - l.requireEqual(t, 2, "mySingleSubData2", "mySingleSubParam") + Fire(evt, "mySingleSubData2") + l.requireEqual(t, 2, "mySingleSubData2") - Unsubscribe(evt) - Raise(evt, "mySingleSubData3") - l.requireEqual(t, 2, "mySingleSubData2", "mySingleSubParam") + RemoveListener(evt, cb) + Fire(evt, "mySingleSubData3") + l.requireEqual(t, 2, "mySingleSubData2") - Subscribe(evt, l, "") - Raise(evt, "mySingleSubData4") - l.requireEqual(t, 3, "mySingleSubData4", "") + AddListener(evt, cb) + Fire(evt, "mySingleSubData4") + l.requireEqual(t, 3, "mySingleSubData4") } -func TestOverwriteSubscription(t *testing.T) { - evt := EventName("testOverwriteSubEvent") +func Test_MultipleSubscriptions(t *testing.T) { + evt := EventName("testMultiSubEvent") l1 := &testListener{} l2 := &testListener{} - - Subscribe(evt, l1, "param1") - Raise(evt, "data1") - l1.requireEqual(t, 1, "data1", "param1") - l2.requireEqual(t, 0, "", "") - - Subscribe(evt, l2, "param2") - Raise(evt, "data2") - l1.requireEqual(t, 1, "data1", "param1") - l2.requireEqual(t, 1, "data2", "param2") + l3 := &testListener{} + cb1 := callback.New(l1) + cb2 := callback.New(l2) + cb3 := callback.New(l3) + + AddListener(evt, cb1) + AddListener(evt, cb2) + AddListener(evt, cb3) + + Fire(evt, "data1") + l1.requireEqual(t, 1, "data1") + l2.requireEqual(t, 1, "data1") + l3.requireEqual(t, 1, "data1") + + Fire(evt, "data2") + l1.requireEqual(t, 2, "data2") + l2.requireEqual(t, 2, "data2") + l3.requireEqual(t, 2, "data2") + + RemoveListener(evt, cb2) + Fire(evt, "data3") + l1.requireEqual(t, 3, "data3") + l2.requireEqual(t, 2, "data2") // Listener 2 removed, should not increment + l3.requireEqual(t, 3, "data3") } func TestMultipleEvents(t *testing.T) { @@ -99,43 +142,53 @@ func TestMultipleEvents(t *testing.T) { evt2 := EventName("testMultiEvt2") l1 := &testListener{} l2 := &testListener{} + cb1 := callback.New(l1) + cb2 := callback.New(l2) - Subscribe(evt1, l1, "p1") - Subscribe(evt2, l2, "p2") + AddListener(evt1, cb1) + AddListener(evt2, cb2) - Raise(evt1, "d1") - l1.requireEqual(t, 1, "d1", "p1") - l2.requireEqual(t, 0, "", "") + Fire(evt1, "data1") + l1.requireEqual(t, 1, "data1") + l2.requireEqual(t, 0, "") - Raise(evt2, "d2") - l1.requireEqual(t, 1, "d1", "p1") - l2.requireEqual(t, 1, "d2", "p2") + Fire(evt2, "data2") + l1.requireEqual(t, 1, "data1") + l2.requireEqual(t, 1, "data2") - Raise(evt1, "d3") - l1.requireEqual(t, 2, "d3", "p1") - l2.requireEqual(t, 1, "d2", "p2") + Fire(evt1, "data3") + l1.requireEqual(t, 2, "data3") + l2.requireEqual(t, 1, "data2") - Raise(evt2, "d4") - l1.requireEqual(t, 2, "d3", "p1") - l2.requireEqual(t, 2, "d4", "p2") + Fire(evt2, "data4") + l1.requireEqual(t, 2, "data3") + l2.requireEqual(t, 2, "data4") } func TestConcurrentEvents(t *testing.T) { - const numEvents = 50 - const invokesPerEvent = 50 - + const ( + numEvents = 50 + listenersPerEvent = 20 + invokesPerEvent = 50 + ) + + originalCnt := len(listeners) + evts := make([]EventName, numEvents) var wg sync.WaitGroup // Subscribe to events concurrently - listeners := make([]*testListener, numEvents) - wg.Add(numEvents) + handlers := make([]*testListener, numEvents*listenersPerEvent) + wg.Add(numEvents * listenersPerEvent) for i := 0; i < numEvents; i++ { - go func(i int) { - defer wg.Done() - listeners[i] = &testListener{} - evtName := EventName(fmt.Sprintf("testConcurrentEvent-%d", i)) - Subscribe(evtName, listeners[i], fmt.Sprintf("param-%d", i)) - }(i) + evts[i] = EventName(fmt.Sprintf("testConcurrentEvent-%d", i)) + for j := 0; j < listenersPerEvent; j++ { + go func(i, j int) { + defer wg.Done() + lis := &testListener{} + AddListener(evts[i], callback.New(lis)) + handlers[i*listenersPerEvent+j] = lis + }(i, j) + } } wg.Wait() @@ -145,39 +198,35 @@ func TestConcurrentEvents(t *testing.T) { for j := 0; j < invokesPerEvent; j++ { go func(i, j int) { defer wg.Done() - evtName := EventName(fmt.Sprintf("testConcurrentEvent-%d", i)) - Raise(evtName, fmt.Sprintf("data-%d-%d", i, j)) + Fire(evts[i], fmt.Sprintf("data-%d-%d", i, j)) }(i, j) } } wg.Wait() // Verify results - for i := 0; i < numEvents; i++ { - require.Equal(t, int32(invokesPerEvent), listeners[i].cnt.Load()) - require.Regexp(t, fmt.Sprintf("data-%d-\\d", i), listeners[i].lastData.Load()) - require.Equal(t, fmt.Sprintf("param-%d", i), listeners[i].lastParam.Load()) + require.Len(t, listeners, originalCnt+numEvents) + for i := 0; i < numEvents*listenersPerEvent; i++ { + require.Equal(t, int32(invokesPerEvent), handlers[i].cnt.Load()) + require.Regexp(t, fmt.Sprintf("data-%d-\\d", i/listenersPerEvent), handlers[i].lastData.Load()) } } type testListener struct { - cnt atomic.Int32 - lastData, lastParam atomic.Value + cnt atomic.Int32 + lastData atomic.Value } -func (l *testListener) Handle(eventData, param string) { +func (l *testListener) OnCall(eventData string) { l.cnt.Add(1) l.lastData.Store(eventData) - l.lastParam.Store(param) } -func (l *testListener) requireEqual(t *testing.T, cnt int32, data string, param string) { +func (l *testListener) requireEqual(t *testing.T, cnt int32, data string) { require.Equal(t, cnt, l.cnt.Load()) if cnt == 0 { require.Nil(t, l.lastData.Load()) - require.Nil(t, l.lastParam.Load()) } else { require.Equal(t, data, l.lastData.Load()) - require.Equal(t, param, l.lastParam.Load()) } } diff --git a/client/go/outline/vpn/vpn.go b/client/go/outline/vpn/vpn.go index 832d118a1bc..1dea4befc83 100644 --- a/client/go/outline/vpn/vpn.go +++ b/client/go/outline/vpn/vpn.go @@ -83,7 +83,7 @@ var conn *VPNConnection func (c *VPNConnection) SetStatus(status ConnectionStatus) { c.Status = status if connJson, err := json.Marshal(c); err == nil { - event.Raise(ConnectionStatusChanged, string(connJson)) + event.Fire(ConnectionStatusChanged, string(connJson)) } else { slog.Warn("failed to marshal VPN connection", "err", err) } From cd3129d97ee7c1e832750911ba773c2bd91006be Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Tue, 21 Jan 2025 18:36:22 -0500 Subject: [PATCH 07/23] expose new callback function in electron Go plugin --- client/go/outline/electron/go_plugin.go | 59 ++++++++++--------------- client/go/outline/method_channel.go | 43 +++++++++++------- 2 files changed, 51 insertions(+), 51 deletions(-) diff --git a/client/go/outline/electron/go_plugin.go b/client/go/outline/electron/go_plugin.go index 0b4317afd0a..40f054681ed 100644 --- a/client/go/outline/electron/go_plugin.go +++ b/client/go/outline/electron/go_plugin.go @@ -30,24 +30,20 @@ typedef struct InvokeMethodResult_t const char *ErrorJson; } InvokeMethodResult; -// ListenerFunc is a C function pointer type that represents a callback function. -// This callback function will be invoked when an event is emitted. +// CallbackFuncPtr is a C function pointer type that represents a callback function. +// This callback function will be invoked by the Go side. // -// - data: The event data, passed as a C string. -// - param: An optional parameter that was passed during [SubscribeEvent], also passed as a C string. -typedef void (*ListenerFunc)(const char *data, const char *param); +// - data: The callback data, passed as a C string (typically a JSON string). +typedef void (*CallbackFuncPtr)(const char *data); -// InvokeListenerFunc takes a ListenerFunc callback, event data, and a parameter, and invokes the -// callback with these arguments. +// InvokeCallback is a helper function that invokes the C callback function pointer. // -// This function is the glue code needed for Go to call C function pointers. +// This function serves as a bridge, allowing Go to call a C function pointer. // // - f: The C function pointer to be invoked. -// - data: The event data, passed as a C string. -// - param: An optional parameter, passed as a C string. -static void InvokeListenerFunc(ListenerFunc f, const char *data, const char *param) -{ - f(data, param); +// - data: A C-string typed data to be passed to the callback. +static void InvokeCallback(CallbackFuncPtr f, const char *data) { + f(data); } */ import "C" @@ -58,7 +54,7 @@ import ( "unsafe" "github.com/Jigsaw-Code/outline-apps/client/go/outline" - "github.com/Jigsaw-Code/outline-apps/client/go/outline/event" + "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" "github.com/Jigsaw-Code/outline-apps/client/go/outline/platerrors" ) @@ -78,34 +74,27 @@ func InvokeMethod(method *C.char, input *C.char) C.InvokeMethodResult { } } -// cgoListener implements [event.Listener] and calls a C function pointer when an event is emitted. -type cgoListener struct { - cb C.ListenerFunc +// cgoCallback implements the [callback.Callback] interface and bridges the Go callback +// to a C function pointer. +type cgoCallback struct { + ptr C.CallbackFuncPtr } -var _ event.Listener = (*cgoListener)(nil) - -// Handle forwards the event data and the parameter to the C callback function pointer. -func (l *cgoListener) Handle(eventData, param string) { - C.InvokeListenerFunc(l.cb, newCGoString(eventData), newCGoString(param)) -} +var _ callback.Callback = (*cgoCallback)(nil) -// SubscribeEvent allows TypeScript to subscribe to events implemented by the event package. -// -// For more details, refer to the documentation of the [event.Subscribe]. -// -//export SubscribeEvent -func SubscribeEvent(eventName *C.char, callback C.ListenerFunc, param *C.char) { - event.Subscribe(event.EventName(C.GoString(eventName)), &cgoListener{callback}, C.GoString(param)) +// OnCall forwards the data to the C callback function pointer. +func (ccb *cgoCallback) OnCall(data string) { + C.InvokeCallback(ccb.ptr, newCGoString(data)) } -// UnsubscribeEvent allows TypeScript to unsubscribe from events. +// NewCallback registers a new callback function and returns a [callback.Token] string. // -// For more details, refer to the documentation of the [event.Unsubscribe]. +// The caller can delete the callback by calling [InvokeMethod] with method "DeleteCallback". // -//export UnsubscribeEvent -func UnsubscribeEvent(eventName *C.char) { - event.Unsubscribe(event.EventName(C.GoString(eventName))) +//export NewCallback +func NewCallback(cb C.CallbackFuncPtr) C.InvokeMethodResult { + token := callback.New(&cgoCallback{cb}) + return C.InvokeMethodResult{Output: newCGoString(string(token))} } // newCGoString allocates memory for a C string based on the given Go string. diff --git a/client/go/outline/method_channel.go b/client/go/outline/method_channel.go index 65500ba4983..bf4326f28fd 100644 --- a/client/go/outline/method_channel.go +++ b/client/go/outline/method_channel.go @@ -17,15 +17,24 @@ package outline import ( "fmt" + "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" "github.com/Jigsaw-Code/outline-apps/client/go/outline/platerrors" ) // API name constants const ( - // FetchResource fetches a resource located at a given URL. - // - Input: the URL string of the resource to fetch - // - Output: the content in raw string of the fetched resource - MethodFetchResource = "FetchResource" + // CloseVPN closes an existing VPN connection and restores network traffic to the default + // network interface. + // + // - Input: null + // - Output: null + MethodCloseVPN = "CloseVPN" + + // DeleteCallback deletes a callback associated with the given callback.Token string. + // + // - Input: The callback token as a string. + // - Output: null + MethodDeleteCallback = "DeleteCallback" // EstablishVPN initiates a VPN connection and directs all network traffic through Outline. // @@ -33,12 +42,10 @@ const ( // - Output: a JSON string of vpn.connectionJSON. MethodEstablishVPN = "EstablishVPN" - // CloseVPN closes an existing VPN connection and restores network traffic to the default - // network interface. - // - // - Input: null - // - Output: null - MethodCloseVPN = "CloseVPN" + // FetchResource fetches a resource located at a given URL. + // - Input: the URL string of the resource to fetch + // - Output: the content in raw string of the fetched resource + MethodFetchResource = "FetchResource" ) // InvokeMethodResult represents the result of an InvokeMethod call. @@ -52,11 +59,9 @@ type InvokeMethodResult struct { // InvokeMethod calls a method by name. func InvokeMethod(method string, input string) *InvokeMethodResult { switch method { - case MethodFetchResource: - url := input - content, err := fetchResource(url) + case MethodCloseVPN: + err := closeVPN() return &InvokeMethodResult{ - Value: content, Error: platerrors.ToPlatformError(err), } @@ -66,9 +71,15 @@ func InvokeMethod(method string, input string) *InvokeMethodResult { Error: platerrors.ToPlatformError(err), } - case MethodCloseVPN: - err := closeVPN() + case MethodDeleteCallback: + callback.Delete(callback.Token(input)) + return &InvokeMethodResult{} + + case MethodFetchResource: + url := input + content, err := fetchResource(url) return &InvokeMethodResult{ + Value: content, Error: platerrors.ToPlatformError(err), } From 5a2456921dfe9b3574a922cf1c00f0504f9e9ea5 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Tue, 21 Jan 2025 20:22:14 -0500 Subject: [PATCH 08/23] Use the new callback in TypeScript --- client/electron/go_plugin.ts | 136 +++++++++--------------- client/electron/vpn_service.ts | 39 +++---- client/go/outline/electron/go_plugin.go | 9 +- client/go/outline/event.go | 62 +++++++++++ client/go/outline/method_channel.go | 39 ++++--- 5 files changed, 163 insertions(+), 122 deletions(-) create mode 100644 client/go/outline/event.go diff --git a/client/electron/go_plugin.ts b/client/electron/go_plugin.ts index 004cc529689..5405f16e32e 100644 --- a/client/electron/go_plugin.ts +++ b/client/electron/go_plugin.ts @@ -38,53 +38,61 @@ export async function invokeGoMethod( const result = await ensureCgo().invokeMethod(method, input); console.debug(`[Backend] - InvokeMethod "${method}" returned`, result); if (result.ErrorJson) { - throw Error(result.ErrorJson); + throw new Error(result.ErrorJson); } return result.Output; } /** - * Represents a callback function for an event. - * @param data The event data string passed from the source. - * @param param The param string provided during registration. + * Represents a function that will be called from the Go backend. + * @param data The data string passed from the Go backend. */ -export type CallbackFunction = (data: string, param: string) => void; +export type CallbackFunction = (data: string) => void; /** - * Subscribes to an event from the Go backend. + * Koffi requires us to register all persistent callbacks; we track the registrations here. + * @see https://koffi.dev/callbacks#registered-callbacks + */ +const koffiCallbacks = new Map(); + +/** + * Registers a callback function in TypeScript, making it invokable from Go. * - * @param name The name of the event to subscribe to. - * @param callback The callback function to be called when the event is fired. - * @param param An optional parameter to be passed to the callback function. - * @returns A Promise that resolves when the subscription is successful. + * The caller can delete the callback by calling `deleteCallback`. * - * @remarks Subscribing to an event will replace any previously subscribed callback for that event. + * @param callback The callback function to be registered. + * @returns A Promise resolves to the callback token, which can be used to refer to the callback. */ -export async function subscribeEvent( - name: string, - callback: CallbackFunction, - param: string | null = null -): Promise { - console.debug(`[Backend] - calling SubscribeEvent "${name}" "${param}" ...`); - await ensureCgo().subscribeEvent( - name, - registerKoffiCallback(name, callback), - param - ); - console.debug(`[Backend] - SubscribeEvent "${name}" done`); +export async function newCallback(callback: CallbackFunction): Promise { + console.debug(`[Backend] - calling newCallback ...`); + const persistentCallback = koffi.register(callback, ensureCgo().callbackFuncPtr); + const result = await ensureCgo().newCallback(persistentCallback); + console.debug(`[Backend] - newCallback done`, result); + if (result.ErrorJson) { + koffi.unregister(persistentCallback); + throw new Error(result.ErrorJson); + } + koffiCallbacks.set(result.Output, persistentCallback); + console.debug(`[Backend] - registered persistent callback ${result.Output} with koffi`); + return result.Output; } /** - * Unsubscribes from an event from the Go backend. + * Unregisters a specified callback function from the Go backend. * - * @param name The name of the event to unsubscribe from. - * @returns A Promise that resolves when the unsubscription is successful. + * @param token The callback token returned from `newCallback`. + * @returns A Promise that resolves when the unregistration is done. */ -export async function unsubscribeEvent(name: string): Promise { - console.debug(`[Backend] - calling UnsubscribeEvent "${name}" ...`); - await ensureCgo().unsubscribeEvent(name); - unregisterKoffiCallback(name); - console.debug(`[Backend] - UnsubscribeEvent "${name}" done`); +export async function deleteCallback(token: string): Promise { + console.debug(`[Backend] - calling deleteCallback ...`); + await ensureCgo().deleteCallback(token); + console.debug(`[Backend] - deleteCallback done`); + const persistentCallback = koffiCallbacks.get(token); + if (persistentCallback) { + koffi.unregister(persistentCallback); + koffiCallbacks.delete(token); + console.debug(`[Backend] - unregistered persistent callback ${token} from koffi`) + } } /** Interface containing the exported native CGo functions. */ @@ -92,14 +100,14 @@ interface CgoFunctions { // InvokeMethodResult InvokeMethod(char* method, char* input); invokeMethod: Function; - // void (*ListenerFunc)(const char *data, const char *param); - listenerFuncPtr: koffi.IKoffiCType; + // void (*CallbackFuncPtr)(const char *data); + callbackFuncPtr: koffi.IKoffiCType; - // void SubscribeEvent(char* eventName, ListenerFunc callback, char* param); - subscribeEvent: Function; + // InvokeMethodResult NewCallback(CallbackFuncPtr cb); + newCallback: Function; - // void UnsubscribeEvent(char* eventName); - unsubscribeEvent: Function; + // void DeleteCallback(char* token); + deleteCallback: Function; } /** Singleton of the loaded native CGo functions. */ @@ -131,59 +139,21 @@ function ensureCgo(): CgoFunctions { backendLib.func('InvokeMethod', invokeMethodResult, ['str', 'str']).async ); - // Define SubscribeEvent/UnsubscribeEvent data structures and function - const listenerFuncPtr = koffi.pointer( - koffi.proto('ListenerFunc', 'void', [cgoString, cgoString]) + // Define callback data structures and functions + const callbackFuncPtr = koffi.pointer( + koffi.proto('CallbackFuncPtr', 'void', [cgoString]) ); - const subscribeEvent = promisify( - backendLib.func('SubscribeEvent', 'void', ['str', listenerFuncPtr, 'str']) + const newCallback = promisify( + backendLib.func('NewCallback', invokeMethodResult, [callbackFuncPtr]) .async ); - const unsubscribeEvent = promisify( - backendLib.func('UnsubscribeEvent', 'void', ['str']).async + const deleteCallback = promisify( + backendLib.func('DeleteCallback', 'void', ['str']).async ); // Cache them so we don't have to reload these functions - cgo = {invokeMethod, listenerFuncPtr, subscribeEvent, unsubscribeEvent}; + cgo = {invokeMethod, callbackFuncPtr, newCallback, deleteCallback}; console.debug('[Backend] - cgo environment initialized'); } return cgo; } - -//#region Koffi's internal registration management - -const koffiCallbacks = new Map(); - -/** - * Registers a persistent JS callback function with Koffi. - * This will replace any previously registered functions to align with `subscribeEvent`. - * - * @param eventName The name of the event. - * @param jsCallback The JavaScript callback function. - * @returns The registered Koffi callback. - * @see https://koffi.dev/callbacks#registered-callbacks - */ -function registerKoffiCallback( - eventName: string, - jsCallback: CallbackFunction -): koffi.IKoffiRegisteredCallback { - unregisterKoffiCallback(eventName); - const koffiCb = koffi.register(jsCallback, ensureCgo().listenerFuncPtr); - koffiCallbacks.set(eventName, koffiCb); - return koffiCb; -} - -/** - * Unregisters a Koffi callback for a specific event. - * - * @param eventName The name of the event. - */ -function unregisterKoffiCallback(eventName: string): void { - const cb = koffiCallbacks.get(eventName); - if (cb) { - koffi.unregister(cb); - koffiCallbacks.delete(eventName); - } -} - -//#endregion Koffi's internal registration management diff --git a/client/electron/vpn_service.ts b/client/electron/vpn_service.ts index c23b8d81386..ce2f6e57352 100644 --- a/client/electron/vpn_service.ts +++ b/client/electron/vpn_service.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {invokeGoMethod} from './go_plugin'; +import {invokeGoMethod, newCallback} from './go_plugin'; import { StartRequestJson, TunnelStatus, @@ -36,7 +36,7 @@ interface EstablishVpnRequest { } export async function establishVpn(request: StartRequestJson) { - subscribeVPNConnEvents(); + await subscribeVPNConnEvents(); const config: EstablishVpnRequest = { vpn: { @@ -68,11 +68,11 @@ export async function establishVpn(request: StartRequestJson) { transport: request.config.transport, }; - await invokeMethod('EstablishVPN', JSON.stringify(config)); + await invokeGoMethod('EstablishVPN', JSON.stringify(config)); } export async function closeVpn(): Promise { - await invokeMethod('CloseVPN', ''); + await invokeGoMethod('CloseVPN', ''); } export type VpnStatusCallback = (id: string, status: TunnelStatus) => void; @@ -88,7 +88,7 @@ export function onVpnStatusChanged(cb: VpnStatusCallback): void { * @param connJson The JSON string representing the VPNConn interface. */ function handleVpnConnectionStatusChanged(connJson: string) { - const conn = parseVPNConnJSON(connJson); + const conn = JSON.parse(connJson) as VPNConn; console.debug(`received ${StatusChangedEvent}`, conn); switch (conn?.status) { case VPNConnConnected: @@ -106,16 +106,20 @@ function handleVpnConnectionStatusChanged(connJson: string) { } } -let vpnEventsSubscribed = false; +// Callback token of the VPNConnStatusChanged event +let vpnConnStatusChangedCb: string | undefined; /** * Subscribes to all VPN connection related events. * This function ensures that the subscription only happens once. */ -function subscribeVPNConnEvents(): void { - if (!vpnEventsSubscribed) { - subscribeEvent(StatusChangedEvent, handleVpnConnectionStatusChanged); - vpnEventsSubscribed = true; +async function subscribeVPNConnEvents(): Promise { + if (!vpnConnStatusChangedCb) { + vpnConnStatusChangedCb = await newCallback(handleVpnConnectionStatusChanged); + await invokeGoMethod('AddEventListener', JSON.stringify({ + name: StatusChangedEvent, + callbackToken: vpnConnStatusChangedCb, + })); } } @@ -137,19 +141,4 @@ interface VPNConn { readonly status: VPNConnStatus; } -function parseVPNConnJSON(json: string): VPNConn | null { - try { - const rawConn = JSON.parse(json); - if (!('id' in rawConn) || !rawConn.id || !('status' in rawConn)) { - return null; - } - return { - id: rawConn.id, - status: rawConn.status, - }; - } catch { - return null; - } -} - //#endregion type definitions of VPNConnection in Go diff --git a/client/go/outline/electron/go_plugin.go b/client/go/outline/electron/go_plugin.go index 40f054681ed..fdeec130ef4 100644 --- a/client/go/outline/electron/go_plugin.go +++ b/client/go/outline/electron/go_plugin.go @@ -89,7 +89,7 @@ func (ccb *cgoCallback) OnCall(data string) { // NewCallback registers a new callback function and returns a [callback.Token] string. // -// The caller can delete the callback by calling [InvokeMethod] with method "DeleteCallback". +// The caller can delete the callback by calling [DeleteCallback] with the returned token. // //export NewCallback func NewCallback(cb C.CallbackFuncPtr) C.InvokeMethodResult { @@ -97,6 +97,13 @@ func NewCallback(cb C.CallbackFuncPtr) C.InvokeMethodResult { return C.InvokeMethodResult{Output: newCGoString(string(token))} } +// DeleteCallback deletes the callback identified by the token returned by [NewCallback]. +// +//export DeleteCallback +func DeleteCallback(token *C.char) { + callback.Delete(callback.Token(C.GoString(token))) +} + // newCGoString allocates memory for a C string based on the given Go string. // It should be paired with [FreeCGoString] to avoid memory leaks. func newCGoString(s string) *C.char { diff --git a/client/go/outline/event.go b/client/go/outline/event.go new file mode 100644 index 00000000000..0091adbaebf --- /dev/null +++ b/client/go/outline/event.go @@ -0,0 +1,62 @@ +// Copyright 2025 The Outline Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package outline + +import ( + "encoding/json" + + "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" + "github.com/Jigsaw-Code/outline-apps/client/go/outline/event" + perrs "github.com/Jigsaw-Code/outline-apps/client/go/outline/platerrors" +) + +// eventListenerJSON represents the JSON structure for adding or removing event listeners. +type eventListenerJSON struct { + // Name is the name of the event to listen for. + Name string `json:"name"` + + // Callback token is the token of the callback function returned from callback.NewCallback. + Callback string `json:"callbackToken"` +} + +func addEventListener(input string) error { + listener, err := parseEventListenerInput(input) + if err != nil { + return err + } + event.AddListener(event.EventName(listener.Name), callback.Token(listener.Callback)) + return nil +} + +func removeEventListener(input string) error { + listener, err := parseEventListenerInput(input) + if err != nil { + return err + } + event.RemoveListener(event.EventName(listener.Name), callback.Token(listener.Callback)) + return nil +} + +func parseEventListenerInput(input string) (*eventListenerJSON, error) { + var listener eventListenerJSON + if err := json.Unmarshal([]byte(input), &listener); err != nil { + return nil, perrs.PlatformError{ + Code: perrs.InternalError, + Message: "invalid event listener argument", + Cause: perrs.ToPlatformError(err), + } + } + return &listener, nil +} diff --git a/client/go/outline/method_channel.go b/client/go/outline/method_channel.go index bf4326f28fd..58023e05de8 100644 --- a/client/go/outline/method_channel.go +++ b/client/go/outline/method_channel.go @@ -17,12 +17,17 @@ package outline import ( "fmt" - "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" "github.com/Jigsaw-Code/outline-apps/client/go/outline/platerrors" ) // API name constants const ( + // AddEventListener registers a callback for a specific event. + // + // - Input: a JSON string of eventListenerJSON + // - Output: null + MethodAddEventListener = "AddEventListener" + // CloseVPN closes an existing VPN connection and restores network traffic to the default // network interface. // @@ -30,22 +35,22 @@ const ( // - Output: null MethodCloseVPN = "CloseVPN" - // DeleteCallback deletes a callback associated with the given callback.Token string. - // - // - Input: The callback token as a string. - // - Output: null - MethodDeleteCallback = "DeleteCallback" - // EstablishVPN initiates a VPN connection and directs all network traffic through Outline. // - // - Input: a JSON string of vpn.configJSON. - // - Output: a JSON string of vpn.connectionJSON. + // - Input: a JSON string of vpnConfigJSON + // - Output: null MethodEstablishVPN = "EstablishVPN" // FetchResource fetches a resource located at a given URL. // - Input: the URL string of the resource to fetch // - Output: the content in raw string of the fetched resource MethodFetchResource = "FetchResource" + + // RemoveEventListener unregisters a callback from a specific event. + // + // - Input: a JSON string of eventListenerJSON + // - Output: null + MethodRemoveEventListener = "RemoveEventListener" ) // InvokeMethodResult represents the result of an InvokeMethod call. @@ -59,6 +64,12 @@ type InvokeMethodResult struct { // InvokeMethod calls a method by name. func InvokeMethod(method string, input string) *InvokeMethodResult { switch method { + case MethodAddEventListener: + err := addEventListener(input) + return &InvokeMethodResult{ + Error: platerrors.ToPlatformError(err), + } + case MethodCloseVPN: err := closeVPN() return &InvokeMethodResult{ @@ -71,10 +82,6 @@ func InvokeMethod(method string, input string) *InvokeMethodResult { Error: platerrors.ToPlatformError(err), } - case MethodDeleteCallback: - callback.Delete(callback.Token(input)) - return &InvokeMethodResult{} - case MethodFetchResource: url := input content, err := fetchResource(url) @@ -83,6 +90,12 @@ func InvokeMethod(method string, input string) *InvokeMethodResult { Error: platerrors.ToPlatformError(err), } + case MethodRemoveEventListener: + err := removeEventListener(input) + return &InvokeMethodResult{ + Error: platerrors.ToPlatformError(err), + } + default: return &InvokeMethodResult{Error: &platerrors.PlatformError{ Code: platerrors.InternalError, From 055c1e323acb56193f2fc2f97f703c0e742686c6 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Wed, 22 Jan 2025 18:36:13 -0500 Subject: [PATCH 09/23] test latest go for CI --- .github/workflows/build_and_test_debug_client.yml | 2 +- client/go/Taskfile.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build_and_test_debug_client.yml b/.github/workflows/build_and_test_debug_client.yml index a43eb874388..b1f3a33410c 100644 --- a/.github/workflows/build_and_test_debug_client.yml +++ b/.github/workflows/build_and_test_debug_client.yml @@ -127,7 +127,7 @@ jobs: - name: Install Go uses: actions/setup-go@v4 with: - go-version-file: '${{ github.workspace }}/go.mod' + go-version: 'stable' - name: Install zig uses: mlugg/setup-zig@v1 diff --git a/client/go/Taskfile.yml b/client/go/Taskfile.yml index 5994e02e1de..54ba6b671c8 100644 --- a/client/go/Taskfile.yml +++ b/client/go/Taskfile.yml @@ -48,7 +48,7 @@ tasks: GOOS={{.TARGET_OS}} GOARCH={{.TARGET_ARCH}} CGO_ENABLED=1 \ CC='zig cc -target {{if eq .TARGET_ARCH "386"}}x86{{else}}x86_64{{end}}-{{.TARGET_OS}}{{if eq .TARGET_OS "linux"}}-gnu{{end}}' {{- end}} \ - go build -trimpath -ldflags="-s -w -X=main.version={{.TUN2SOCKS_VERSION}}" -o '{{.OUTPUT}}' '{{.TASKFILE_DIR}}/outline/electron' + go env && go build -trimpath -x -v -ldflags="-s -w -X=main.version={{.TUN2SOCKS_VERSION}}" -o '{{.OUTPUT}}' '{{.TASKFILE_DIR}}/outline/electron' - | {{if or (ne OS .TARGET_OS) (ne ARCH .TARGET_ARCH) -}} GOOS={{.TARGET_OS}} GOARCH={{.TARGET_ARCH}} CGO_ENABLED=1 \ From 22ebf3c94cf794a49da85c7a5feeabefa08fd250 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Wed, 22 Jan 2025 18:58:41 -0500 Subject: [PATCH 10/23] try gnu clib --- client/go/Taskfile.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/go/Taskfile.yml b/client/go/Taskfile.yml index 54ba6b671c8..d1c272e5ff0 100644 --- a/client/go/Taskfile.yml +++ b/client/go/Taskfile.yml @@ -46,13 +46,13 @@ tasks: - | {{if or (ne OS .TARGET_OS) (ne ARCH .TARGET_ARCH) -}} GOOS={{.TARGET_OS}} GOARCH={{.TARGET_ARCH}} CGO_ENABLED=1 \ - CC='zig cc -target {{if eq .TARGET_ARCH "386"}}x86{{else}}x86_64{{end}}-{{.TARGET_OS}}{{if eq .TARGET_OS "linux"}}-gnu{{end}}' + CC='zig cc -target {{if eq .TARGET_ARCH "386"}}x86{{else}}x86_64{{end}}-{{.TARGET_OS}}-gnu' {{- end}} \ - go env && go build -trimpath -x -v -ldflags="-s -w -X=main.version={{.TUN2SOCKS_VERSION}}" -o '{{.OUTPUT}}' '{{.TASKFILE_DIR}}/outline/electron' + go build -trimpath -x -v -ldflags="-s -w -X=main.version={{.TUN2SOCKS_VERSION}}" -o '{{.OUTPUT}}' '{{.TASKFILE_DIR}}/outline/electron' - | {{if or (ne OS .TARGET_OS) (ne ARCH .TARGET_ARCH) -}} GOOS={{.TARGET_OS}} GOARCH={{.TARGET_ARCH}} CGO_ENABLED=1 \ - CC='zig cc -target {{if eq .TARGET_ARCH "386"}}x86{{else}}x86_64{{end}}-{{.TARGET_OS}}{{if eq .TARGET_OS "linux"}}-gnu{{end}}' + CC='zig cc -target {{if eq .TARGET_ARCH "386"}}x86{{else}}x86_64{{end}}-{{.TARGET_OS}}-gnu' {{- end}} \ go build -trimpath -buildmode=c-shared -ldflags="-s -w -X=main.version={{.TUN2SOCKS_VERSION}}" -o '{{.OUTPUT_LIB}}' '{{.TASKFILE_DIR}}/outline/electron' From 8c4ffacf50d3a153c835ead43b5a11da42b7efe0 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Wed, 22 Jan 2025 19:25:23 -0500 Subject: [PATCH 11/23] more debug for zig --- .github/workflows/build_and_test_debug_client.yml | 5 +++-- client/go/Taskfile.yml | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build_and_test_debug_client.yml b/.github/workflows/build_and_test_debug_client.yml index b1f3a33410c..d099cd87976 100644 --- a/.github/workflows/build_and_test_debug_client.yml +++ b/.github/workflows/build_and_test_debug_client.yml @@ -131,8 +131,9 @@ jobs: - name: Install zig uses: mlugg/setup-zig@v1 - with: - version: 0.13.0 + + - name: Build Go First (Test) + run: npm run action client/go/build windows - name: Build Windows Client run: npm run action client/electron/build windows diff --git a/client/go/Taskfile.yml b/client/go/Taskfile.yml index d1c272e5ff0..604bb49768a 100644 --- a/client/go/Taskfile.yml +++ b/client/go/Taskfile.yml @@ -48,7 +48,7 @@ tasks: GOOS={{.TARGET_OS}} GOARCH={{.TARGET_ARCH}} CGO_ENABLED=1 \ CC='zig cc -target {{if eq .TARGET_ARCH "386"}}x86{{else}}x86_64{{end}}-{{.TARGET_OS}}-gnu' {{- end}} \ - go build -trimpath -x -v -ldflags="-s -w -X=main.version={{.TUN2SOCKS_VERSION}}" -o '{{.OUTPUT}}' '{{.TASKFILE_DIR}}/outline/electron' + go build -trimpath -ldflags="-s -w -X=main.version={{.TUN2SOCKS_VERSION}}" -o '{{.OUTPUT}}' '{{.TASKFILE_DIR}}/outline/electron' - | {{if or (ne OS .TARGET_OS) (ne ARCH .TARGET_ARCH) -}} GOOS={{.TARGET_OS}} GOARCH={{.TARGET_ARCH}} CGO_ENABLED=1 \ From d72486b5f55a4ad8333aae4130dda8532ae48a56 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Wed, 22 Jan 2025 19:37:51 -0500 Subject: [PATCH 12/23] Try newer action runner --- .github/workflows/build_and_test_debug_client.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build_and_test_debug_client.yml b/.github/workflows/build_and_test_debug_client.yml index d099cd87976..31f70d526bd 100644 --- a/.github/workflows/build_and_test_debug_client.yml +++ b/.github/workflows/build_and_test_debug_client.yml @@ -104,7 +104,7 @@ jobs: windows_debug_build: name: Windows Debug Build - runs-on: windows-2019 + runs-on: windows-2022 timeout-minutes: 20 needs: [web_test, backend_test] steps: From 4429a105d290afa4f639fa1ead9ead0e7039b704 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Wed, 22 Jan 2025 19:58:49 -0500 Subject: [PATCH 13/23] try mingw --- .github/workflows/build_and_test_debug_client.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/build_and_test_debug_client.yml b/.github/workflows/build_and_test_debug_client.yml index 31f70d526bd..bf1a31957a2 100644 --- a/.github/workflows/build_and_test_debug_client.yml +++ b/.github/workflows/build_and_test_debug_client.yml @@ -129,6 +129,11 @@ jobs: with: go-version: 'stable' + - name: Set up MinGW + uses: egor-tensin/setup-mingw@v2 + with: + platform: x86 + - name: Install zig uses: mlugg/setup-zig@v1 From 93496cbcb3dce76058d98b7cc3770f6142ddbd3e Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Thu, 23 Jan 2025 15:00:50 -0500 Subject: [PATCH 14/23] revert the CI job changes and clear cache --- .github/workflows/build_and_test_debug_client.yml | 14 ++++---------- client/go/Taskfile.yml | 4 ++-- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build_and_test_debug_client.yml b/.github/workflows/build_and_test_debug_client.yml index bf1a31957a2..a43eb874388 100644 --- a/.github/workflows/build_and_test_debug_client.yml +++ b/.github/workflows/build_and_test_debug_client.yml @@ -104,7 +104,7 @@ jobs: windows_debug_build: name: Windows Debug Build - runs-on: windows-2022 + runs-on: windows-2019 timeout-minutes: 20 needs: [web_test, backend_test] steps: @@ -127,18 +127,12 @@ jobs: - name: Install Go uses: actions/setup-go@v4 with: - go-version: 'stable' - - - name: Set up MinGW - uses: egor-tensin/setup-mingw@v2 - with: - platform: x86 + go-version-file: '${{ github.workspace }}/go.mod' - name: Install zig uses: mlugg/setup-zig@v1 - - - name: Build Go First (Test) - run: npm run action client/go/build windows + with: + version: 0.13.0 - name: Build Windows Client run: npm run action client/electron/build windows diff --git a/client/go/Taskfile.yml b/client/go/Taskfile.yml index 604bb49768a..5994e02e1de 100644 --- a/client/go/Taskfile.yml +++ b/client/go/Taskfile.yml @@ -46,13 +46,13 @@ tasks: - | {{if or (ne OS .TARGET_OS) (ne ARCH .TARGET_ARCH) -}} GOOS={{.TARGET_OS}} GOARCH={{.TARGET_ARCH}} CGO_ENABLED=1 \ - CC='zig cc -target {{if eq .TARGET_ARCH "386"}}x86{{else}}x86_64{{end}}-{{.TARGET_OS}}-gnu' + CC='zig cc -target {{if eq .TARGET_ARCH "386"}}x86{{else}}x86_64{{end}}-{{.TARGET_OS}}{{if eq .TARGET_OS "linux"}}-gnu{{end}}' {{- end}} \ go build -trimpath -ldflags="-s -w -X=main.version={{.TUN2SOCKS_VERSION}}" -o '{{.OUTPUT}}' '{{.TASKFILE_DIR}}/outline/electron' - | {{if or (ne OS .TARGET_OS) (ne ARCH .TARGET_ARCH) -}} GOOS={{.TARGET_OS}} GOARCH={{.TARGET_ARCH}} CGO_ENABLED=1 \ - CC='zig cc -target {{if eq .TARGET_ARCH "386"}}x86{{else}}x86_64{{end}}-{{.TARGET_OS}}-gnu' + CC='zig cc -target {{if eq .TARGET_ARCH "386"}}x86{{else}}x86_64{{end}}-{{.TARGET_OS}}{{if eq .TARGET_OS "linux"}}-gnu{{end}}' {{- end}} \ go build -trimpath -buildmode=c-shared -ldflags="-s -w -X=main.version={{.TUN2SOCKS_VERSION}}" -o '{{.OUTPUT_LIB}}' '{{.TASKFILE_DIR}}/outline/electron' From f33903ee9538e391e7b71cba7ebdd71803f7aaf4 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Thu, 23 Jan 2025 16:42:10 -0500 Subject: [PATCH 15/23] add callback Token type to TypeScript --- client/electron/go_plugin.ts | 34 ++++++++++++++++++++++------------ client/electron/vpn_service.ts | 4 ++-- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/client/electron/go_plugin.ts b/client/electron/go_plugin.ts index 5405f16e32e..0fcb24f9412 100644 --- a/client/electron/go_plugin.ts +++ b/client/electron/go_plugin.ts @@ -12,11 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {promisify} from 'node:util'; +import { promisify } from 'node:util'; import koffi from 'koffi'; -import {pathToBackendLibrary} from './app_paths'; +import { pathToBackendLibrary } from './app_paths'; /** * Calls a Go function by invoking the `InvokeMethod` function in the native backend library. @@ -49,6 +49,9 @@ export async function invokeGoMethod( */ export type CallbackFunction = (data: string) => void; +/** A unique token for a callback created by `newCallback`. */ +export type CallbackToken = string; + /** * Koffi requires us to register all persistent callbacks; we track the registrations here. * @see https://koffi.dev/callbacks#registered-callbacks @@ -63,17 +66,22 @@ const koffiCallbacks = new Map(); * @param callback The callback function to be registered. * @returns A Promise resolves to the callback token, which can be used to refer to the callback. */ -export async function newCallback(callback: CallbackFunction): Promise { - console.debug(`[Backend] - calling newCallback ...`); - const persistentCallback = koffi.register(callback, ensureCgo().callbackFuncPtr); +export async function newCallback(callback: CallbackFunction): Promise { + console.debug('[Backend] - calling newCallback ...'); + const persistentCallback = koffi.register( + callback, + ensureCgo().callbackFuncPtr + ); const result = await ensureCgo().newCallback(persistentCallback); - console.debug(`[Backend] - newCallback done`, result); + console.debug('[Backend] - newCallback done', result); if (result.ErrorJson) { koffi.unregister(persistentCallback); throw new Error(result.ErrorJson); } koffiCallbacks.set(result.Output, persistentCallback); - console.debug(`[Backend] - registered persistent callback ${result.Output} with koffi`); + console.debug( + `[Backend] - registered persistent callback ${result.Output} with koffi` + ); return result.Output; } @@ -83,15 +91,17 @@ export async function newCallback(callback: CallbackFunction): Promise { * @param token The callback token returned from `newCallback`. * @returns A Promise that resolves when the unregistration is done. */ -export async function deleteCallback(token: string): Promise { - console.debug(`[Backend] - calling deleteCallback ...`); +export async function deleteCallback(token: CallbackToken): Promise { + console.debug('[Backend] - calling deleteCallback ...'); await ensureCgo().deleteCallback(token); - console.debug(`[Backend] - deleteCallback done`); + console.debug('[Backend] - deleteCallback done'); const persistentCallback = koffiCallbacks.get(token); if (persistentCallback) { koffi.unregister(persistentCallback); koffiCallbacks.delete(token); - console.debug(`[Backend] - unregistered persistent callback ${token} from koffi`) + console.debug( + `[Backend] - unregistered persistent callback ${token} from koffi` + ); } } @@ -152,7 +162,7 @@ function ensureCgo(): CgoFunctions { ); // Cache them so we don't have to reload these functions - cgo = {invokeMethod, callbackFuncPtr, newCallback, deleteCallback}; + cgo = { invokeMethod, callbackFuncPtr, newCallback, deleteCallback }; console.debug('[Backend] - cgo environment initialized'); } return cgo; diff --git a/client/electron/vpn_service.ts b/client/electron/vpn_service.ts index ce2f6e57352..ff10e15280f 100644 --- a/client/electron/vpn_service.ts +++ b/client/electron/vpn_service.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {invokeGoMethod, newCallback} from './go_plugin'; +import { CallbackToken, invokeGoMethod, newCallback } from './go_plugin'; import { StartRequestJson, TunnelStatus, @@ -107,7 +107,7 @@ function handleVpnConnectionStatusChanged(connJson: string) { } // Callback token of the VPNConnStatusChanged event -let vpnConnStatusChangedCb: string | undefined; +let vpnConnStatusChangedCb: CallbackToken | undefined; /** * Subscribes to all VPN connection related events. From 2e126df4e6fd7cc211927205b837e784d2d26a39 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Thu, 23 Jan 2025 17:12:58 -0500 Subject: [PATCH 16/23] fix linting errors --- client/electron/go_plugin.ts | 10 ++++++---- client/electron/vpn_service.ts | 17 +++++++++++------ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/client/electron/go_plugin.ts b/client/electron/go_plugin.ts index 0fcb24f9412..1ef1ef25625 100644 --- a/client/electron/go_plugin.ts +++ b/client/electron/go_plugin.ts @@ -12,11 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { promisify } from 'node:util'; +import {promisify} from 'node:util'; import koffi from 'koffi'; -import { pathToBackendLibrary } from './app_paths'; +import {pathToBackendLibrary} from './app_paths'; /** * Calls a Go function by invoking the `InvokeMethod` function in the native backend library. @@ -66,7 +66,9 @@ const koffiCallbacks = new Map(); * @param callback The callback function to be registered. * @returns A Promise resolves to the callback token, which can be used to refer to the callback. */ -export async function newCallback(callback: CallbackFunction): Promise { +export async function newCallback( + callback: CallbackFunction +): Promise { console.debug('[Backend] - calling newCallback ...'); const persistentCallback = koffi.register( callback, @@ -162,7 +164,7 @@ function ensureCgo(): CgoFunctions { ); // Cache them so we don't have to reload these functions - cgo = { invokeMethod, callbackFuncPtr, newCallback, deleteCallback }; + cgo = {invokeMethod, callbackFuncPtr, newCallback, deleteCallback}; console.debug('[Backend] - cgo environment initialized'); } return cgo; diff --git a/client/electron/vpn_service.ts b/client/electron/vpn_service.ts index ff10e15280f..3d878a55545 100644 --- a/client/electron/vpn_service.ts +++ b/client/electron/vpn_service.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { CallbackToken, invokeGoMethod, newCallback } from './go_plugin'; +import {CallbackToken, invokeGoMethod, newCallback} from './go_plugin'; import { StartRequestJson, TunnelStatus, @@ -115,11 +115,16 @@ let vpnConnStatusChangedCb: CallbackToken | undefined; */ async function subscribeVPNConnEvents(): Promise { if (!vpnConnStatusChangedCb) { - vpnConnStatusChangedCb = await newCallback(handleVpnConnectionStatusChanged); - await invokeGoMethod('AddEventListener', JSON.stringify({ - name: StatusChangedEvent, - callbackToken: vpnConnStatusChangedCb, - })); + vpnConnStatusChangedCb = await newCallback( + handleVpnConnectionStatusChanged + ); + await invokeGoMethod( + 'AddEventListener', + JSON.stringify({ + name: StatusChangedEvent, + callbackToken: vpnConnStatusChangedCb, + }) + ); } } From 508087944b96c158437733e131b9001b944fc981 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Mon, 27 Jan 2025 15:32:08 -0500 Subject: [PATCH 17/23] Use callback Token directly --- client/go/outline/callback/callback.go | 49 ++++++--------------- client/go/outline/callback/callback_test.go | 40 +++++++---------- client/go/outline/electron/go_plugin.go | 10 ++--- client/go/outline/event.go | 2 +- client/go/outline/event/event.go | 7 ++- client/go/outline/event/event_test.go | 4 +- 6 files changed, 43 insertions(+), 69 deletions(-) diff --git a/client/go/outline/callback/callback.go b/client/go/outline/callback/callback.go index 6d2975b5b15..08385e9e73b 100644 --- a/client/go/outline/callback/callback.go +++ b/client/go/outline/callback/callback.go @@ -16,13 +16,12 @@ package callback import ( - "fmt" "log/slog" "sync" ) // Token can be used to uniquely identify a registered callback. -type Token string +type Token int // Callback is an interface that can be implemented to receive callbacks. type Callback interface { @@ -31,8 +30,8 @@ type Callback interface { var ( mu sync.RWMutex - callbacks = make(map[uint32]Callback) - nextCbID uint32 = 1 + callbacks = make(map[Token]Callback) + nextCbID Token = 1 ) // New registers a new callback and returns a unique callback token. @@ -40,11 +39,11 @@ func New(c Callback) Token { mu.Lock() defer mu.Unlock() - id := nextCbID + token := nextCbID nextCbID++ - callbacks[id] = c - slog.Debug("callback created", "id", id) - return getTokenByID(id) + callbacks[token] = c + slog.Debug("callback created", "token", token) + return token } // Delete removes a callback identified by the token. @@ -54,44 +53,22 @@ func Delete(token Token) { mu.Lock() defer mu.Unlock() - if id, err := getIDByToken(token); err == nil { - delete(callbacks, id) - slog.Debug("callback deleted", "id", id) - } else { - slog.Warn("invalid callback token", "err", err, "token", token) - } + delete(callbacks, token) + slog.Debug("callback deleted", "token", token) } // Call executes a callback identified by the token. // // Calling this function is safe even if the callback has not been registered. func Call(token Token, data string) { - id, err := getIDByToken(token) - if err != nil { - slog.Warn("invalid callback token", "err", err, "token", token) - return - } - mu.RLock() - cb, ok := callbacks[id] - mu.RUnlock() + defer mu.RUnlock() + cb, ok := callbacks[token] if !ok { - slog.Warn("callback not yet created", "id", id, "token", token) + slog.Warn("callback not yet created", "token", token) return } - slog.Debug("invoking callback", "id", id, "data", data) + slog.Debug("invoking callback", "token", token, "data", data) cb.OnCall(data) } - -// getTokenByID creates a string-based callback token from a number-based internal ID. -func getTokenByID(id uint32) Token { - return Token(fmt.Sprintf("cbid-%d", id)) -} - -// getIDByToken parses a number-based internal ID from a string-based callback token. -func getIDByToken(token Token) (uint32, error) { - var id uint32 - _, err := fmt.Sscanf(string(token), "cbid-%d", &id) - return id, err -} diff --git a/client/go/outline/callback/callback_test.go b/client/go/outline/callback/callback_test.go index 81b371fbfd7..88fcec67257 100644 --- a/client/go/outline/callback/callback_test.go +++ b/client/go/outline/callback/callback_test.go @@ -26,33 +26,31 @@ import ( func Test_New(t *testing.T) { curID := nextCbID token := New(&testCallback{}) + require.Equal(t, curID, token) + require.Contains(t, callbacks, token) require.Equal(t, curID+1, nextCbID) - require.Contains(t, callbacks, curID) - - require.NotEmpty(t, token) - require.Equal(t, fmt.Sprintf("cbid-%d", curID), string(token)) - - id, err := getIDByToken(token) - require.NoError(t, err) - require.Contains(t, callbacks, id) - require.Equal(t, id, curID) } func Test_Delete(t *testing.T) { curID := nextCbID token := New(&testCallback{}) - require.Contains(t, callbacks, curID) + require.Equal(t, curID, token) + require.Contains(t, callbacks, token) Delete(token) - require.NotContains(t, callbacks, curID) + require.NotContains(t, callbacks, token) require.Equal(t, curID+1, nextCbID) - Delete("invalid-token") - require.NotContains(t, callbacks, curID) + Delete(0) + require.NotContains(t, callbacks, token) require.Equal(t, curID+1, nextCbID) - Delete("cbid-99999999") - require.NotContains(t, callbacks, curID) + Delete(-1) + require.NotContains(t, callbacks, token) + require.Equal(t, curID+1, nextCbID) + + Delete(99999999) + require.NotContains(t, callbacks, token) require.Equal(t, curID+1, nextCbID) } @@ -64,13 +62,13 @@ func Test_Call(t *testing.T) { Call(token, "arg1") c.requireEqual(t, 1, "arg1") - Call("invalid-token", "arg1") + Call(-1, "arg1") c.requireEqual(t, 1, "arg1") // No change Call(token, "arg2") c.requireEqual(t, 2, "arg2") - Call("cbid-99999999", "arg3") + Call(99999999, "arg3") c.requireEqual(t, 2, "arg2") // No change } @@ -87,8 +85,7 @@ func Test_ConcurrentCreate(t *testing.T) { go func(i int) { defer wg.Done() tokens[i] = New(&testCallback{}) - require.NotEmpty(t, tokens[i]) - require.Regexp(t, `^cbid-\d+$`, tokens[i]) + require.Greater(t, tokens[i], 0) }(i) } wg.Wait() @@ -99,10 +96,7 @@ func Test_ConcurrentCreate(t *testing.T) { for _, token := range tokens { require.False(t, tokenSet[token], "Duplicate token found: %s", token) tokenSet[token] = true - - id, err := getIDByToken(token) - require.NoError(t, err) - require.Contains(t, callbacks, id) + require.Contains(t, callbacks, token) } } diff --git a/client/go/outline/electron/go_plugin.go b/client/go/outline/electron/go_plugin.go index fdeec130ef4..6c5b76ab13e 100644 --- a/client/go/outline/electron/go_plugin.go +++ b/client/go/outline/electron/go_plugin.go @@ -87,21 +87,21 @@ func (ccb *cgoCallback) OnCall(data string) { C.InvokeCallback(ccb.ptr, newCGoString(data)) } -// NewCallback registers a new callback function and returns a [callback.Token] string. +// NewCallback registers a new callback function and returns a [callback.Token] number. // // The caller can delete the callback by calling [DeleteCallback] with the returned token. // //export NewCallback -func NewCallback(cb C.CallbackFuncPtr) C.InvokeMethodResult { +func NewCallback(cb C.CallbackFuncPtr) C.int { token := callback.New(&cgoCallback{cb}) - return C.InvokeMethodResult{Output: newCGoString(string(token))} + return C.int(token) } // DeleteCallback deletes the callback identified by the token returned by [NewCallback]. // //export DeleteCallback -func DeleteCallback(token *C.char) { - callback.Delete(callback.Token(C.GoString(token))) +func DeleteCallback(token C.int) { + callback.Delete(callback.Token(token)) } // newCGoString allocates memory for a C string based on the given Go string. diff --git a/client/go/outline/event.go b/client/go/outline/event.go index 0091adbaebf..765927aa66f 100644 --- a/client/go/outline/event.go +++ b/client/go/outline/event.go @@ -28,7 +28,7 @@ type eventListenerJSON struct { Name string `json:"name"` // Callback token is the token of the callback function returned from callback.NewCallback. - Callback string `json:"callbackToken"` + Callback int `json:"callbackToken"` } func addEventListener(input string) error { diff --git a/client/go/outline/event/event.go b/client/go/outline/event/event.go index c29c2dcf830..30b1658b3dc 100644 --- a/client/go/outline/event/event.go +++ b/client/go/outline/event/event.go @@ -36,8 +36,8 @@ var ( // // The provided callback will be called when the event is invoked, along with the event data. func AddListener(evt EventName, cb callback.Token) { - if evt == "" || cb == "" { - slog.Warn("empty event or callback are ignored") + if evt == "" || cb <= 0 { + slog.Warn("ignores empty event or invalid callback token") return } mu.Lock() @@ -62,6 +62,9 @@ func RemoveListener(evt EventName, cb callback.Token) { // Fire triggers the specified [EventName], invoking all associated callbacks with the given data. // +// The event data string must contain at least a sender ID and the event details. +// This allows listeners to identify who triggered the event. +// // Calling this function is safe even if the event has not been registered. func Fire(evt EventName, data string) { mu.RLock() diff --git a/client/go/outline/event/event_test.go b/client/go/outline/event/event_test.go index 97fb0471fcf..2e95c8272cf 100644 --- a/client/go/outline/event/event_test.go +++ b/client/go/outline/event/event_test.go @@ -60,10 +60,10 @@ func Test_ZeroParams(t *testing.T) { l := &testListener{} AddListener("", callback.New(l)) Fire("", "") - AddListener("TestEvent_InvalidParam", "") + AddListener("TestEvent_InvalidParam", -1) RemoveListener("", callback.New(l)) - RemoveListener("TestEvent_InvalidParam", "") + RemoveListener("TestEvent_InvalidParam", -1) RemoveListener("NonExistEventName", callback.New(&testListener{})) Fire("", "data1") From 4e3e12024081b44d4f38fa9fda1a9e3d7ba5a517 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Mon, 27 Jan 2025 16:12:08 -0500 Subject: [PATCH 18/23] refactor VPN status changed event --- client/electron/go_plugin.ts | 30 +++---- client/electron/index.ts | 20 +++-- client/electron/vpn_service.ts | 86 +++++++++------------ client/go/outline/callback/callback.go | 12 ++- client/go/outline/callback/callback_test.go | 18 +++-- client/go/outline/electron/go_plugin.go | 11 +-- client/go/outline/event/event_test.go | 3 +- 7 files changed, 92 insertions(+), 88 deletions(-) diff --git a/client/electron/go_plugin.ts b/client/electron/go_plugin.ts index 1ef1ef25625..e5c7195ef95 100644 --- a/client/electron/go_plugin.ts +++ b/client/electron/go_plugin.ts @@ -46,17 +46,18 @@ export async function invokeGoMethod( /** * Represents a function that will be called from the Go backend. * @param data The data string passed from the Go backend. + * @returns A result string that will be passed back to the caller. */ -export type CallbackFunction = (data: string) => void; +export type CallbackFunction = (data: string) => string; /** A unique token for a callback created by `newCallback`. */ -export type CallbackToken = string; +export type CallbackToken = number; /** * Koffi requires us to register all persistent callbacks; we track the registrations here. * @see https://koffi.dev/callbacks#registered-callbacks */ -const koffiCallbacks = new Map(); +const koffiCallbacks = new Map(); /** * Registers a callback function in TypeScript, making it invokable from Go. @@ -74,17 +75,10 @@ export async function newCallback( callback, ensureCgo().callbackFuncPtr ); - const result = await ensureCgo().newCallback(persistentCallback); - console.debug('[Backend] - newCallback done', result); - if (result.ErrorJson) { - koffi.unregister(persistentCallback); - throw new Error(result.ErrorJson); - } - koffiCallbacks.set(result.Output, persistentCallback); - console.debug( - `[Backend] - registered persistent callback ${result.Output} with koffi` - ); - return result.Output; + const token = await ensureCgo().newCallback(persistentCallback); + console.debug('[Backend] - newCallback done', token); + koffiCallbacks.set(token, persistentCallback); + return token.Output; } /** @@ -94,7 +88,7 @@ export async function newCallback( * @returns A Promise that resolves when the unregistration is done. */ export async function deleteCallback(token: CallbackToken): Promise { - console.debug('[Backend] - calling deleteCallback ...'); + console.debug('[Backend] - calling deleteCallback ...', token); await ensureCgo().deleteCallback(token); console.debug('[Backend] - deleteCallback done'); const persistentCallback = koffiCallbacks.get(token); @@ -153,14 +147,14 @@ function ensureCgo(): CgoFunctions { // Define callback data structures and functions const callbackFuncPtr = koffi.pointer( - koffi.proto('CallbackFuncPtr', 'void', [cgoString]) + koffi.proto('CallbackFuncPtr', 'str', [cgoString]) ); const newCallback = promisify( - backendLib.func('NewCallback', invokeMethodResult, [callbackFuncPtr]) + backendLib.func('NewCallback', 'int', [callbackFuncPtr]) .async ); const deleteCallback = promisify( - backendLib.func('DeleteCallback', 'void', ['str']).async + backendLib.func('DeleteCallback', 'void', ['int']).async ); // Cache them so we don't have to reload these functions diff --git a/client/electron/index.ts b/client/electron/index.ts index 303a66b341a..9bffe712522 100644 --- a/client/electron/index.ts +++ b/client/electron/index.ts @@ -59,6 +59,7 @@ declare const APP_VERSION: string; const debugMode = process.env.OUTLINE_DEBUG === 'true'; const IS_LINUX = os.platform() === 'linux'; +const USE_MODERN_ROUTING = (IS_LINUX && !process.env.APPIMAGE); // Used for the auto-connect feature. There will be a tunnel in store // if the user was connected at shutdown. @@ -369,15 +370,20 @@ async function createVpnTunnel( return tunnel; } +/** + * Initializes the VPN service. For example, subscribing to global events. + */ +async function initVpnService() { + if (USE_MODERN_ROUTING) { + onVpnStatusChanged((id, status) => setUiTunnelStatus(status, id)); + } +} + // Invoked by both the start-proxying event handler and auto-connect. async function startVpn(request: StartRequestJson, isAutoConnect: boolean) { console.debug('startVpn called with request ', JSON.stringify(request)); - if (IS_LINUX && !process.env.APPIMAGE) { - onVpnStatusChanged((id, status) => { - setUiTunnelStatus(status, id); - console.info('VPN Status Changed: ', id, status); - }); + if (USE_MODERN_ROUTING) { await establishVpn(request); return; } @@ -415,7 +421,7 @@ async function startVpn(request: StartRequestJson, isAutoConnect: boolean) { // Invoked by both the stop-proxying event and quit handler. async function stopVpn() { - if (IS_LINUX && !process.env.APPIMAGE) { + if (USE_MODERN_ROUTING) { await Promise.all([closeVpn(), tearDownAutoLaunch()]); return; } @@ -471,6 +477,8 @@ function main() { // TODO(fortuna): Start the app with the window hidden on auto-start? setupWindow(); + await initVpnService(); + let requestAtShutdown: StartRequestJson | undefined; try { requestAtShutdown = await tunnelStore.load(); diff --git a/client/electron/vpn_service.ts b/client/electron/vpn_service.ts index 3d878a55545..b1c69c1f04b 100644 --- a/client/electron/vpn_service.ts +++ b/client/electron/vpn_service.ts @@ -36,8 +36,6 @@ interface EstablishVpnRequest { } export async function establishVpn(request: StartRequestJson) { - await subscribeVPNConnEvents(); - const config: EstablishVpnRequest = { vpn: { id: request.id, @@ -77,55 +75,47 @@ export async function closeVpn(): Promise { export type VpnStatusCallback = (id: string, status: TunnelStatus) => void; -let statusCb: VpnStatusCallback | undefined = undefined; - -export function onVpnStatusChanged(cb: VpnStatusCallback): void { - statusCb = cb; -} - /** - * Forwards the VPN connection status change event to the VpnStatusCallback. - * @param connJson The JSON string representing the VPNConn interface. + * Registers a callback function to be invoked when the VPN status changes. + * + * @param cb - The callback function to be invoked when the VPN status changes. + * The callback will receive the VPN connection ID as well as the new status. + * + * @remarks The caller should subscribe to this event **only once**. + * Use the `id` parameter in the callback to identify the firing VPN connection. */ -function handleVpnConnectionStatusChanged(connJson: string) { - const conn = JSON.parse(connJson) as VPNConn; - console.debug(`received ${StatusChangedEvent}`, conn); - switch (conn?.status) { - case VPNConnConnected: - statusCb?.(conn.id, TunnelStatus.CONNECTED); - break; - case VPNConnConnecting: - statusCb?.(conn.id, TunnelStatus.RECONNECTING); - break; - case VPNConnDisconnecting: - statusCb?.(conn.id, TunnelStatus.DISCONNECTING); - break; - case VPNConnDisconnected: - statusCb?.(conn.id, TunnelStatus.DISCONNECTED); - break; +export async function onVpnStatusChanged(cb: VpnStatusCallback): Promise { + if (!cb) { + return; } -} -// Callback token of the VPNConnStatusChanged event -let vpnConnStatusChangedCb: CallbackToken | undefined; - -/** - * Subscribes to all VPN connection related events. - * This function ensures that the subscription only happens once. - */ -async function subscribeVPNConnEvents(): Promise { - if (!vpnConnStatusChangedCb) { - vpnConnStatusChangedCb = await newCallback( - handleVpnConnectionStatusChanged - ); - await invokeGoMethod( - 'AddEventListener', - JSON.stringify({ - name: StatusChangedEvent, - callbackToken: vpnConnStatusChangedCb, - }) - ); - } + const cbToken = await newCallback(data => { + const conn = JSON.parse(data) as VPNConnectionState; + console.debug(`received ${StatusChangedEvent}`, conn); + switch (conn?.status) { + case VPNConnConnected: + cb(conn.id, TunnelStatus.CONNECTED); + break; + case VPNConnConnecting: + cb(conn.id, TunnelStatus.RECONNECTING); + break; + case VPNConnDisconnecting: + cb(conn.id, TunnelStatus.DISCONNECTING); + break; + case VPNConnDisconnected: + cb(conn.id, TunnelStatus.DISCONNECTED); + break; + } + return ""; + }); + + await invokeGoMethod( + 'AddEventListener', + JSON.stringify({ + name: StatusChangedEvent, + callbackToken: cbToken, + }) + ); } //#region type definitions of VPNConnection in Go @@ -141,7 +131,7 @@ const VPNConnConnected: VPNConnStatus = 'Connected'; const VPNConnDisconnecting: VPNConnStatus = 'Disconnecting'; const VPNConnDisconnected: VPNConnStatus = 'Disconnected'; -interface VPNConn { +interface VPNConnectionState { readonly id: string; readonly status: VPNConnStatus; } diff --git a/client/go/outline/callback/callback.go b/client/go/outline/callback/callback.go index 08385e9e73b..b985279ab4f 100644 --- a/client/go/outline/callback/callback.go +++ b/client/go/outline/callback/callback.go @@ -24,8 +24,10 @@ import ( type Token int // Callback is an interface that can be implemented to receive callbacks. +// +// It accepts an input and returns an output, allowing communication back to the caller. type Callback interface { - OnCall(data string) + OnCall(data string) string } var ( @@ -59,16 +61,18 @@ func Delete(token Token) { // Call executes a callback identified by the token. // +// It passes the data string to the [Callback].OnCall and returns the string returned by OnCall. +// // Calling this function is safe even if the callback has not been registered. -func Call(token Token, data string) { +func Call(token Token, data string) string { mu.RLock() defer mu.RUnlock() cb, ok := callbacks[token] if !ok { slog.Warn("callback not yet created", "token", token) - return + return "" } slog.Debug("invoking callback", "token", token, "data", data) - cb.OnCall(data) + return cb.OnCall(data) } diff --git a/client/go/outline/callback/callback_test.go b/client/go/outline/callback/callback_test.go index 88fcec67257..6805b519df7 100644 --- a/client/go/outline/callback/callback_test.go +++ b/client/go/outline/callback/callback_test.go @@ -59,16 +59,20 @@ func Test_Call(t *testing.T) { token := New(c) c.requireEqual(t, 0, "") - Call(token, "arg1") + ret := Call(token, "arg1") + require.Equal(t, ret, "ret-arg1") c.requireEqual(t, 1, "arg1") - Call(-1, "arg1") + ret = Call(-1, "arg1") + require.Empty(t, ret) c.requireEqual(t, 1, "arg1") // No change - Call(token, "arg2") + ret = Call(token, "arg2") + require.Equal(t, ret, "ret-arg2") c.requireEqual(t, 2, "arg2") - Call(99999999, "arg3") + ret = Call(99999999, "arg3") + require.Empty(t, ret) c.requireEqual(t, 2, "arg2") // No change } @@ -114,7 +118,8 @@ func Test_ConcurrentCall(t *testing.T) { for i := 0; i < numInvocations; i++ { go func(i int) { defer wg.Done() - Call(token, fmt.Sprintf("data-%d", i)) + ret := Call(token, fmt.Sprintf("data-%d", i)) + require.Equal(t, ret, fmt.Sprintf("ret-data-%d", i)) }(i) } wg.Wait() @@ -162,9 +167,10 @@ type testCallback struct { lastData atomic.Value } -func (tc *testCallback) OnCall(data string) { +func (tc *testCallback) OnCall(data string) string { tc.cnt.Add(1) tc.lastData.Store(data) + return fmt.Sprintf("ret-%s", data) } func (tc *testCallback) requireEqual(t *testing.T, cnt int32, data string) { diff --git a/client/go/outline/electron/go_plugin.go b/client/go/outline/electron/go_plugin.go index 6c5b76ab13e..51f61b9e5e7 100644 --- a/client/go/outline/electron/go_plugin.go +++ b/client/go/outline/electron/go_plugin.go @@ -34,7 +34,7 @@ typedef struct InvokeMethodResult_t // This callback function will be invoked by the Go side. // // - data: The callback data, passed as a C string (typically a JSON string). -typedef void (*CallbackFuncPtr)(const char *data); +typedef const char* (*CallbackFuncPtr)(const char *data); // InvokeCallback is a helper function that invokes the C callback function pointer. // @@ -42,8 +42,8 @@ typedef void (*CallbackFuncPtr)(const char *data); // // - f: The C function pointer to be invoked. // - data: A C-string typed data to be passed to the callback. -static void InvokeCallback(CallbackFuncPtr f, const char *data) { - f(data); +static const char* InvokeCallback(CallbackFuncPtr f, const char *data) { + return f(data); } */ import "C" @@ -83,8 +83,9 @@ type cgoCallback struct { var _ callback.Callback = (*cgoCallback)(nil) // OnCall forwards the data to the C callback function pointer. -func (ccb *cgoCallback) OnCall(data string) { - C.InvokeCallback(ccb.ptr, newCGoString(data)) +func (ccb *cgoCallback) OnCall(data string) string { + ret := C.InvokeCallback(ccb.ptr, newCGoString(data)) + return C.GoString(ret) } // NewCallback registers a new callback function and returns a [callback.Token] number. diff --git a/client/go/outline/event/event_test.go b/client/go/outline/event/event_test.go index 2e95c8272cf..36a5da1be43 100644 --- a/client/go/outline/event/event_test.go +++ b/client/go/outline/event/event_test.go @@ -217,9 +217,10 @@ type testListener struct { lastData atomic.Value } -func (l *testListener) OnCall(eventData string) { +func (l *testListener) OnCall(eventData string) string { l.cnt.Add(1) l.lastData.Store(eventData) + return "" } func (l *testListener) requireEqual(t *testing.T, cnt int32, data string) { From b8e738f55f34b4145dd40305e2d85a0f3349fb68 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Mon, 27 Jan 2025 16:50:43 -0500 Subject: [PATCH 19/23] Fix linting --- client/electron/go_plugin.ts | 9 ++++----- client/electron/index.ts | 2 +- client/electron/vpn_service.ts | 6 +++--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/client/electron/go_plugin.ts b/client/electron/go_plugin.ts index e5c7195ef95..05f251589a5 100644 --- a/client/electron/go_plugin.ts +++ b/client/electron/go_plugin.ts @@ -76,9 +76,9 @@ export async function newCallback( ensureCgo().callbackFuncPtr ); const token = await ensureCgo().newCallback(persistentCallback); - console.debug('[Backend] - newCallback done', token); + console.debug('[Backend] - newCallback done, token:', token); koffiCallbacks.set(token, persistentCallback); - return token.Output; + return token; } /** @@ -88,7 +88,7 @@ export async function newCallback( * @returns A Promise that resolves when the unregistration is done. */ export async function deleteCallback(token: CallbackToken): Promise { - console.debug('[Backend] - calling deleteCallback ...', token); + console.debug('[Backend] - calling deleteCallback:', token); await ensureCgo().deleteCallback(token); console.debug('[Backend] - deleteCallback done'); const persistentCallback = koffiCallbacks.get(token); @@ -150,8 +150,7 @@ function ensureCgo(): CgoFunctions { koffi.proto('CallbackFuncPtr', 'str', [cgoString]) ); const newCallback = promisify( - backendLib.func('NewCallback', 'int', [callbackFuncPtr]) - .async + backendLib.func('NewCallback', 'int', [callbackFuncPtr]).async ); const deleteCallback = promisify( backendLib.func('DeleteCallback', 'void', ['int']).async diff --git a/client/electron/index.ts b/client/electron/index.ts index 9bffe712522..ea538d96504 100644 --- a/client/electron/index.ts +++ b/client/electron/index.ts @@ -59,7 +59,7 @@ declare const APP_VERSION: string; const debugMode = process.env.OUTLINE_DEBUG === 'true'; const IS_LINUX = os.platform() === 'linux'; -const USE_MODERN_ROUTING = (IS_LINUX && !process.env.APPIMAGE); +const USE_MODERN_ROUTING = IS_LINUX && !process.env.APPIMAGE; // Used for the auto-connect feature. There will be a tunnel in store // if the user was connected at shutdown. diff --git a/client/electron/vpn_service.ts b/client/electron/vpn_service.ts index b1c69c1f04b..3a6ba52936a 100644 --- a/client/electron/vpn_service.ts +++ b/client/electron/vpn_service.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {CallbackToken, invokeGoMethod, newCallback} from './go_plugin'; +import {invokeGoMethod, newCallback} from './go_plugin'; import { StartRequestJson, TunnelStatus, @@ -78,7 +78,7 @@ export type VpnStatusCallback = (id: string, status: TunnelStatus) => void; /** * Registers a callback function to be invoked when the VPN status changes. * - * @param cb - The callback function to be invoked when the VPN status changes. + * @param cb - The callback function to be invoked when the VPN status changes. * The callback will receive the VPN connection ID as well as the new status. * * @remarks The caller should subscribe to this event **only once**. @@ -106,7 +106,7 @@ export async function onVpnStatusChanged(cb: VpnStatusCallback): Promise { cb(conn.id, TunnelStatus.DISCONNECTED); break; } - return ""; + return ''; }); await invokeGoMethod( From a3de6338f8c78d4f720a1b92db8993535c0a77d9 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Thu, 30 Jan 2025 23:38:18 -0500 Subject: [PATCH 20/23] remove event package, replace it directly with callback --- client/electron/go_plugin.ts | 181 ++++++++------- client/go/outline/callback/callback.go | 89 +++++--- client/go/outline/callback/callback_test.go | 100 ++++----- client/go/outline/electron/go_plugin.go | 24 +- client/go/outline/event.go | 62 ------ client/go/outline/event/event.go | 77 ------- client/go/outline/event/event_test.go | 233 -------------------- client/go/outline/method_channel.go | 26 +-- client/go/outline/vpn/vpn.go | 17 +- client/go/outline/vpn_linux.go | 15 ++ client/go/outline/vpn_others.go | 5 +- 11 files changed, 264 insertions(+), 565 deletions(-) delete mode 100644 client/go/outline/event.go delete mode 100644 client/go/outline/event/event.go delete mode 100644 client/go/outline/event/event_test.go diff --git a/client/electron/go_plugin.ts b/client/electron/go_plugin.ts index 05f251589a5..7caddd5538f 100644 --- a/client/electron/go_plugin.ts +++ b/client/electron/go_plugin.ts @@ -35,7 +35,7 @@ export async function invokeGoMethod( input: string ): Promise { console.debug(`[Backend] - calling InvokeMethod "${method}" ...`); - const result = await ensureCgo().invokeMethod(method, input); + const result = await getDefaultBackendChannel().invokeMethod(method, input); console.debug(`[Backend] - InvokeMethod "${method}" returned`, result); if (result.ErrorJson) { throw new Error(result.ErrorJson); @@ -50,115 +50,146 @@ export async function invokeGoMethod( */ export type CallbackFunction = (data: string) => string; -/** A unique token for a callback created by `newCallback`. */ +/** A token to uniquely identify a callback. */ export type CallbackToken = number; -/** - * Koffi requires us to register all persistent callbacks; we track the registrations here. - * @see https://koffi.dev/callbacks#registered-callbacks - */ -const koffiCallbacks = new Map(); - /** * Registers a callback function in TypeScript, making it invokable from Go. * - * The caller can delete the callback by calling `deleteCallback`. + * The caller can unregister the callback by calling `unregisterCallback`. * * @param callback The callback function to be registered. * @returns A Promise resolves to the callback token, which can be used to refer to the callback. */ -export async function newCallback( +export async function registerCallback( callback: CallbackFunction ): Promise { - console.debug('[Backend] - calling newCallback ...'); - const persistentCallback = koffi.register( - callback, - ensureCgo().callbackFuncPtr - ); - const token = await ensureCgo().newCallback(persistentCallback); - console.debug('[Backend] - newCallback done, token:', token); - koffiCallbacks.set(token, persistentCallback); + console.debug('[Backend] - calling registerCallback ...'); + const token = await getDefaultCallbackManager().register(callback); + console.debug('[Backend] - registerCallback done, token:', token); return token; } /** - * Unregisters a specified callback function from the Go backend. + * Unregisters a specified callback function from the Go backend to release resources. * - * @param token The callback token returned from `newCallback`. + * @param token The callback token returned from `registerCallback`. * @returns A Promise that resolves when the unregistration is done. */ -export async function deleteCallback(token: CallbackToken): Promise { - console.debug('[Backend] - calling deleteCallback:', token); - await ensureCgo().deleteCallback(token); - console.debug('[Backend] - deleteCallback done'); - const persistentCallback = koffiCallbacks.get(token); - if (persistentCallback) { - koffi.unregister(persistentCallback); - koffiCallbacks.delete(token); - console.debug( - `[Backend] - unregistered persistent callback ${token} from koffi` - ); +export async function unregisterCallback(token: CallbackToken): Promise { + console.debug('[Backend] - calling unregisterCallback, token:', token); + await getDefaultCallbackManager().unregister(token); + console.debug('[Backend] - unregisterCallback done, token:', token); +} + +/** Singleton of the CGo backend channel. */ +let callback: CallbackManager | undefined; + +function getDefaultCallbackManager(): CallbackManager { + if (!callback) { + callback = new CallbackManager(getDefaultBackendChannel()); } + return callback; } -/** Interface containing the exported native CGo functions. */ -interface CgoFunctions { - // InvokeMethodResult InvokeMethod(char* method, char* input); - invokeMethod: Function; +class CallbackManager { + /** `void (*CallbackFuncPtr)(const char *data);` */ + private readonly callbackFuncPtr: koffi.IKoffiCType; + + /** `int RegisterCallback(CallbackFuncPtr cb);` */ + private readonly registerCallback: Function; + + /** `void UnregisterCallback(int token);` */ + private readonly unregisterCallback: Function; + + /** + * Koffi requires us to register all persistent callbacks; we track the registrations here. + * @see https://koffi.dev/callbacks#registered-callbacks + */ + private readonly koffiCallbacks = new Map< + CallbackToken, + koffi.IKoffiRegisteredCallback + >(); + + constructor(backend: BackendChannel) { + this.callbackFuncPtr = koffi.pointer( + koffi.proto('CallbackFuncPtr', 'str', [backend.cgoString]) + ); + this.registerCallback = backend.declareCGoFunction( + 'RegisterCallback', + 'int', + [this.callbackFuncPtr] + ); + this.unregisterCallback = backend.declareCGoFunction( + 'UnregisterCallback', + 'void', + ['int'] + ); + } + + async register(callback: CallbackFunction): Promise { + const persistentCallback = koffi.register(callback, this.callbackFuncPtr); + const token = await this.registerCallback(persistentCallback); + this.koffiCallbacks.set(token, persistentCallback); + return token; + } - // void (*CallbackFuncPtr)(const char *data); - callbackFuncPtr: koffi.IKoffiCType; + async unregister(token: CallbackToken): Promise { + await this.unregisterCallback(token); + const persistentCallback = this.koffiCallbacks.get(token); + if (persistentCallback) { + koffi.unregister(persistentCallback); + this.koffiCallbacks.delete(token); + } + } +} - // InvokeMethodResult NewCallback(CallbackFuncPtr cb); - newCallback: Function; +/** Singleton of the CGo backend channel. */ +let backend: BackendChannel | undefined; - // void DeleteCallback(char* token); - deleteCallback: Function; +function getDefaultBackendChannel(): BackendChannel { + if (!backend) { + backend = new BackendChannel(); + } + return backend; } -/** Singleton of the loaded native CGo functions. */ -let cgo: CgoFunctions | undefined; +class BackendChannel { + /** The backend library instance of koffi */ + private readonly library: koffi.IKoffiLib; -/** - * Ensures that the CGo functions are loaded and initialized, returning the singleton instance. - * - * @returns The loaded CGo functions singleton. - */ -function ensureCgo(): CgoFunctions { - if (!cgo) { - console.debug('[Backend] - initializing cgo environment ...'); - const backendLib = koffi.load(pathToBackendLibrary()); + /** An auto releasable `const char *` type in koffi */ + readonly cgoString: koffi.IKoffiCType; - // Define C strings and setup auto release - const cgoString = koffi.disposable( + /** `InvokeMethodResult InvokeMethod(char* method, char* input);` */ + readonly invokeMethod: Function; + + constructor() { + this.library = koffi.load(pathToBackendLibrary()); + + // Define shared types + this.cgoString = koffi.disposable( 'CGoAutoReleaseString', 'str', - backendLib.func('FreeCGoString', 'void', ['str']) + this.library.func('FreeCGoString', 'void', ['str']) ); - // Define InvokeMethod data structures and function const invokeMethodResult = koffi.struct('InvokeMethodResult', { - Output: cgoString, - ErrorJson: cgoString, + Output: this.cgoString, + ErrorJson: this.cgoString, }); - const invokeMethod = promisify( - backendLib.func('InvokeMethod', invokeMethodResult, ['str', 'str']).async - ); - - // Define callback data structures and functions - const callbackFuncPtr = koffi.pointer( - koffi.proto('CallbackFuncPtr', 'str', [cgoString]) - ); - const newCallback = promisify( - backendLib.func('NewCallback', 'int', [callbackFuncPtr]).async - ); - const deleteCallback = promisify( - backendLib.func('DeleteCallback', 'void', ['int']).async + this.invokeMethod = this.declareCGoFunction( + 'InvokeMethod', + invokeMethodResult, + ['str', 'str'] ); + } - // Cache them so we don't have to reload these functions - cgo = {invokeMethod, callbackFuncPtr, newCallback, deleteCallback}; - console.debug('[Backend] - cgo environment initialized'); + declareCGoFunction( + name: string, + result: koffi.TypeSpec, + args: koffi.TypeSpec[] + ): Function { + return promisify(this.library.func(name, result, args)); } - return cgo; } diff --git a/client/go/outline/callback/callback.go b/client/go/outline/callback/callback.go index b985279ab4f..ecc16b5ac1d 100644 --- a/client/go/outline/callback/callback.go +++ b/client/go/outline/callback/callback.go @@ -21,56 +21,85 @@ import ( ) // Token can be used to uniquely identify a registered callback. +// +// This token is designed to be used across language boundaries. +// For example, TypeScript code can use this token to reference a callback. type Token int -// Callback is an interface that can be implemented to receive callbacks. -// -// It accepts an input and returns an output, allowing communication back to the caller. -type Callback interface { +// Handler is an interface that can be implemented to receive callbacks. +type Handler interface { + // OnCall is called when the callback is invoked. It accepts an input string and + // optionally returns an output string. OnCall(data string) string } -var ( +// Manager manages the registration, unregistration, and invocation of callbacks. +type Manager struct { mu sync.RWMutex - callbacks = make(map[Token]Callback) - nextCbID Token = 1 + callbacks map[Token]Handler + nextCbID Token +} + +// variables defining the DefaultManager. +var ( + mgrInstance *Manager + initMgrOnce sync.Once ) -// New registers a new callback and returns a unique callback token. -func New(c Callback) Token { - mu.Lock() - defer mu.Unlock() +// DefaultManager returns the shared default callback [Manager] that can be used across +// all compoenents. +func DefaultManager() *Manager { + initMgrOnce.Do(func() { + mgrInstance = NewManager() + }) + return mgrInstance +} + +// NewManager creates a new callback [Manager]. +func NewManager() *Manager { + return &Manager{ + callbacks: make(map[Token]Handler), + nextCbID: 1, + } +} + +// Register registers a new callback to the [Manager]. +// +// It returns a unique [Token] that can be used to unregister or invoke the callback. +func (m *Manager) Register(c Handler) Token { + m.mu.Lock() + defer m.mu.Unlock() - token := nextCbID - nextCbID++ - callbacks[token] = c - slog.Debug("callback created", "token", token) + token := m.nextCbID + m.nextCbID++ + m.callbacks[token] = c + slog.Debug("callback created", "manager", m, "token", token) return token } -// Delete removes a callback identified by the token. +// Unregister removes a previously registered callback from the [Manager]. // -// Calling this function is safe even if the callback has not been registered. -func Delete(token Token) { - mu.Lock() - defer mu.Unlock() +// It is safe to call this function with a non-registered [Token]. +func (m *Manager) Unregister(token Token) { + m.mu.Lock() + defer m.mu.Unlock() - delete(callbacks, token) - slog.Debug("callback deleted", "token", token) + delete(m.callbacks, token) + slog.Debug("callback deleted", "manager", m, "token", token) } -// Call executes a callback identified by the token. +// Call invokes the callback identified by the given [Token]. // -// It passes the data string to the [Callback].OnCall and returns the string returned by OnCall. +// It passes data to the [Handler]'s OnCall method and returns the result. // -// Calling this function is safe even if the callback has not been registered. -func Call(token Token, data string) string { - mu.RLock() - defer mu.RUnlock() +// It is safe to call this function with a non-registered [Token]. +func (m *Manager) Call(token Token, data string) string { + m.mu.RLock() + defer m.mu.RUnlock() - cb, ok := callbacks[token] + cb, ok := m.callbacks[token] if !ok { - slog.Warn("callback not yet created", "token", token) + slog.Warn("callback not yet registered", "token", token) return "" } slog.Debug("invoking callback", "token", token, "data", data) diff --git a/client/go/outline/callback/callback_test.go b/client/go/outline/callback/callback_test.go index 6805b519df7..69430e3685b 100644 --- a/client/go/outline/callback/callback_test.go +++ b/client/go/outline/callback/callback_test.go @@ -23,64 +23,64 @@ import ( "github.com/stretchr/testify/require" ) -func Test_New(t *testing.T) { - curID := nextCbID - token := New(&testCallback{}) - require.Equal(t, curID, token) - require.Contains(t, callbacks, token) - require.Equal(t, curID+1, nextCbID) +func Test_Register(t *testing.T) { + mgr := NewManager() + token := mgr.Register(&testCallback{}) + require.Equal(t, Token(1), token) + require.Contains(t, mgr.callbacks, token) + require.Equal(t, Token(2), mgr.nextCbID) } -func Test_Delete(t *testing.T) { - curID := nextCbID - token := New(&testCallback{}) - require.Equal(t, curID, token) - require.Contains(t, callbacks, token) +func Test_Unregister(t *testing.T) { + mgr := NewManager() + token := mgr.Register(&testCallback{}) + require.Equal(t, Token(1), token) + require.Contains(t, mgr.callbacks, token) - Delete(token) - require.NotContains(t, callbacks, token) - require.Equal(t, curID+1, nextCbID) + mgr.Unregister(token) + require.NotContains(t, mgr.callbacks, token) + require.Equal(t, Token(2), mgr.nextCbID) - Delete(0) - require.NotContains(t, callbacks, token) - require.Equal(t, curID+1, nextCbID) + mgr.Unregister(0) + require.NotContains(t, mgr.callbacks, token) + require.Equal(t, Token(2), mgr.nextCbID) - Delete(-1) - require.NotContains(t, callbacks, token) - require.Equal(t, curID+1, nextCbID) + mgr.Unregister(-1) + require.NotContains(t, mgr.callbacks, token) + require.Equal(t, Token(2), mgr.nextCbID) - Delete(99999999) - require.NotContains(t, callbacks, token) - require.Equal(t, curID+1, nextCbID) + mgr.Unregister(99999999) + require.NotContains(t, mgr.callbacks, token) + require.Equal(t, Token(2), mgr.nextCbID) } func Test_Call(t *testing.T) { + mgr := NewManager() c := &testCallback{} - token := New(c) + token := mgr.Register(c) c.requireEqual(t, 0, "") - ret := Call(token, "arg1") + ret := mgr.Call(token, "arg1") require.Equal(t, ret, "ret-arg1") c.requireEqual(t, 1, "arg1") - ret = Call(-1, "arg1") + ret = mgr.Call(-1, "arg1") require.Empty(t, ret) c.requireEqual(t, 1, "arg1") // No change - ret = Call(token, "arg2") + ret = mgr.Call(token, "arg2") require.Equal(t, ret, "ret-arg2") c.requireEqual(t, 2, "arg2") - ret = Call(99999999, "arg3") + ret = mgr.Call(99999999, "arg3") require.Empty(t, ret) c.requireEqual(t, 2, "arg2") // No change } -func Test_ConcurrentCreate(t *testing.T) { +func Test_ConcurrentRegister(t *testing.T) { const numTokens = 1000 - curID := nextCbID - originalLen := len(callbacks) + mgr := NewManager() var wg sync.WaitGroup tokens := make([]Token, numTokens) @@ -88,37 +88,35 @@ func Test_ConcurrentCreate(t *testing.T) { for i := 0; i < numTokens; i++ { go func(i int) { defer wg.Done() - tokens[i] = New(&testCallback{}) + tokens[i] = mgr.Register(&testCallback{}) require.Greater(t, tokens[i], 0) }(i) } wg.Wait() - require.Len(t, callbacks, originalLen+numTokens) - require.Equal(t, curID+numTokens, nextCbID) + require.Len(t, mgr.callbacks, numTokens) + require.Equal(t, Token(numTokens+1), mgr.nextCbID) tokenSet := make(map[Token]bool) for _, token := range tokens { require.False(t, tokenSet[token], "Duplicate token found: %s", token) tokenSet[token] = true - require.Contains(t, callbacks, token) + require.Contains(t, mgr.callbacks, token) } } func Test_ConcurrentCall(t *testing.T) { const numInvocations = 1000 - curID := nextCbID - originalLen := len(callbacks) - + mgr := NewManager() c := &testCallback{} - token := New(c) + token := mgr.Register(c) var wg sync.WaitGroup wg.Add(numInvocations) for i := 0; i < numInvocations; i++ { go func(i int) { defer wg.Done() - ret := Call(token, fmt.Sprintf("data-%d", i)) + ret := mgr.Call(token, fmt.Sprintf("data-%d", i)) require.Equal(t, ret, fmt.Sprintf("ret-data-%d", i)) }(i) } @@ -127,38 +125,36 @@ func Test_ConcurrentCall(t *testing.T) { require.Equal(t, int32(numInvocations), c.cnt.Load()) require.Regexp(t, `^data-\d+$`, c.lastData.Load()) - require.Len(t, callbacks, originalLen+1) - require.Equal(t, curID+1, nextCbID) + require.Len(t, mgr.callbacks, 1) + require.Equal(t, Token(2), mgr.nextCbID) } -func Test_ConcurrentDelete(t *testing.T) { +func Test_ConcurrentUnregister(t *testing.T) { const ( numTokens = 50 numDeletes = 1000 ) - curID := nextCbID - originalLen := len(callbacks) - + mgr := NewManager() tokens := make([]Token, numTokens) for i := 0; i < numTokens; i++ { - tokens[i] = New(&testCallback{}) + tokens[i] = mgr.Register(&testCallback{}) } - require.Len(t, callbacks, originalLen+numTokens) - require.Equal(t, curID+numTokens, nextCbID) + require.Len(t, mgr.callbacks, numTokens) + require.Equal(t, Token(numTokens+1), mgr.nextCbID) var wg sync.WaitGroup wg.Add(numDeletes) for i := 0; i < numDeletes; i++ { go func(i int) { defer wg.Done() - Delete(tokens[i%numTokens]) + mgr.Unregister(tokens[i%numTokens]) }(i) } wg.Wait() - require.Len(t, callbacks, originalLen) - require.Equal(t, curID+numTokens, nextCbID) + require.Len(t, mgr.callbacks, 0) + require.Equal(t, Token(numTokens+1), mgr.nextCbID) } // testCallback is a mock implementation of callback.Callback for testing. diff --git a/client/go/outline/electron/go_plugin.go b/client/go/outline/electron/go_plugin.go index 51f61b9e5e7..3f048b3752f 100644 --- a/client/go/outline/electron/go_plugin.go +++ b/client/go/outline/electron/go_plugin.go @@ -74,13 +74,13 @@ func InvokeMethod(method *C.char, input *C.char) C.InvokeMethodResult { } } -// cgoCallback implements the [callback.Callback] interface and bridges the Go callback +// cgoCallback implements the [callback.Handler] interface and bridges the Go callback // to a C function pointer. type cgoCallback struct { ptr C.CallbackFuncPtr } -var _ callback.Callback = (*cgoCallback)(nil) +var _ callback.Handler = (*cgoCallback)(nil) // OnCall forwards the data to the C callback function pointer. func (ccb *cgoCallback) OnCall(data string) string { @@ -88,21 +88,23 @@ func (ccb *cgoCallback) OnCall(data string) string { return C.GoString(ret) } -// NewCallback registers a new callback function and returns a [callback.Token] number. +// RegisterCallback registers a new callback function with the [callback.DefaultManager] +// and returns a [callback.Token] number. // -// The caller can delete the callback by calling [DeleteCallback] with the returned token. +// The caller can delete the callback by calling [UnregisterCallback] with the returned token. // -//export NewCallback -func NewCallback(cb C.CallbackFuncPtr) C.int { - token := callback.New(&cgoCallback{cb}) +//export RegisterCallback +func RegisterCallback(cb C.CallbackFuncPtr) C.int { + token := callback.DefaultManager().Register(&cgoCallback{cb}) return C.int(token) } -// DeleteCallback deletes the callback identified by the token returned by [NewCallback]. +// UnregisterCallback unregisters the callback from the [callback.DefaultManager] +// identified by the token returned by [RegisterCallback]. // -//export DeleteCallback -func DeleteCallback(token C.int) { - callback.Delete(callback.Token(token)) +//export UnregisterCallback +func UnregisterCallback(token C.int) { + callback.DefaultManager().Unregister(callback.Token(token)) } // newCGoString allocates memory for a C string based on the given Go string. diff --git a/client/go/outline/event.go b/client/go/outline/event.go deleted file mode 100644 index 765927aa66f..00000000000 --- a/client/go/outline/event.go +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright 2025 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package outline - -import ( - "encoding/json" - - "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" - "github.com/Jigsaw-Code/outline-apps/client/go/outline/event" - perrs "github.com/Jigsaw-Code/outline-apps/client/go/outline/platerrors" -) - -// eventListenerJSON represents the JSON structure for adding or removing event listeners. -type eventListenerJSON struct { - // Name is the name of the event to listen for. - Name string `json:"name"` - - // Callback token is the token of the callback function returned from callback.NewCallback. - Callback int `json:"callbackToken"` -} - -func addEventListener(input string) error { - listener, err := parseEventListenerInput(input) - if err != nil { - return err - } - event.AddListener(event.EventName(listener.Name), callback.Token(listener.Callback)) - return nil -} - -func removeEventListener(input string) error { - listener, err := parseEventListenerInput(input) - if err != nil { - return err - } - event.RemoveListener(event.EventName(listener.Name), callback.Token(listener.Callback)) - return nil -} - -func parseEventListenerInput(input string) (*eventListenerJSON, error) { - var listener eventListenerJSON - if err := json.Unmarshal([]byte(input), &listener); err != nil { - return nil, perrs.PlatformError{ - Code: perrs.InternalError, - Message: "invalid event listener argument", - Cause: perrs.ToPlatformError(err), - } - } - return &listener, nil -} diff --git a/client/go/outline/event/event.go b/client/go/outline/event/event.go deleted file mode 100644 index 30b1658b3dc..00000000000 --- a/client/go/outline/event/event.go +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2025 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package event provides a thread-safe mechanism for managing and triggering events. -package event - -import ( - "log/slog" - "slices" - "sync" - - "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" -) - -// EventName is a type alias for string that represents the name of an event. -// Using a dedicated type improves type safety when working with event names. -type EventName string - -var ( - mu sync.RWMutex - listeners = make(map[EventName][]callback.Token) -) - -// AddListener adds a [callback.Callback] to a given [EventName]. -// -// The provided callback will be called when the event is invoked, along with the event data. -func AddListener(evt EventName, cb callback.Token) { - if evt == "" || cb <= 0 { - slog.Warn("ignores empty event or invalid callback token") - return - } - mu.Lock() - defer mu.Unlock() - - listeners[evt] = append(listeners[evt], cb) - slog.Debug("successfully subscribed to event", "event", evt, "callback", cb) -} - -// RemoveListener removes a [callback.Callback] from the specified [EventName]. -// -// Calling this function is safe even if the event has not been registered. -func RemoveListener(evt EventName, cb callback.Token) { - mu.Lock() - defer mu.Unlock() - - if cbs, ok := listeners[evt]; ok { - listeners[evt] = slices.DeleteFunc(cbs, func(t callback.Token) bool { return t == cb }) - } - slog.Debug("successfully ubsubscribed from event", "event", evt, "callback", cb) -} - -// Fire triggers the specified [EventName], invoking all associated callbacks with the given data. -// -// The event data string must contain at least a sender ID and the event details. -// This allows listeners to identify who triggered the event. -// -// Calling this function is safe even if the event has not been registered. -func Fire(evt EventName, data string) { - mu.RLock() - defer mu.RUnlock() - - slog.Debug("firing event", "event", evt, "data", data) - for _, cb := range listeners[evt] { - callback.Call(cb, data) - } -} diff --git a/client/go/outline/event/event_test.go b/client/go/outline/event/event_test.go deleted file mode 100644 index 36a5da1be43..00000000000 --- a/client/go/outline/event/event_test.go +++ /dev/null @@ -1,233 +0,0 @@ -// Copyright 2025 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package event - -import ( - "fmt" - "sync" - "sync/atomic" - "testing" - - "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" - "github.com/stretchr/testify/require" -) - -func Test_AddListener(t *testing.T) { - originalCnt := len(listeners) - evt := EventName("TestAddListenerEvent") - cb := callback.New(&testListener{}) - AddListener(evt, cb) - - require.Len(t, listeners, originalCnt+1) - require.Len(t, listeners[evt], 1) - require.Equal(t, listeners[evt][0], cb) -} - -func Test_RemoveListener(t *testing.T) { - originalCnt := len(listeners) - evt := EventName("TestRemoveListenerEvent") - cb1 := callback.New(&testListener{}) - cb2 := callback.New(&testListener{}) - - AddListener(evt, cb1) - AddListener(evt, cb2) - require.Len(t, listeners, originalCnt+1) - require.Len(t, listeners[evt], 2) - require.Equal(t, listeners[evt][0], cb1) - require.Equal(t, listeners[evt][1], cb2) - - RemoveListener(evt, cb1) - require.Len(t, listeners, originalCnt+1) - require.Len(t, listeners[evt], 1) - require.Equal(t, listeners[evt][0], cb2) -} - -func Test_ZeroParams(t *testing.T) { - originalCnt := len(listeners) - - l := &testListener{} - AddListener("", callback.New(l)) - Fire("", "") - AddListener("TestEvent_InvalidParam", -1) - - RemoveListener("", callback.New(l)) - RemoveListener("TestEvent_InvalidParam", -1) - RemoveListener("NonExistEventName", callback.New(&testListener{})) - - Fire("", "data1") - Fire("NonExistEventName", "data2") - - require.Len(t, listeners, originalCnt) -} - -func Test_NoSubscription(t *testing.T) { - evt := EventName("testNoSubEvent") - l := &testListener{} - cb := callback.New(l) - - Fire(evt, "data") - l.requireEqual(t, 0, "") // No listener, callback should not be called - - AddListener(evt, cb) - RemoveListener(evt, cb) - Fire(evt, "data2") - l.requireEqual(t, 0, "") // Listener removed, callback should not be called -} - -func Test_SingleSubscription(t *testing.T) { - evt := EventName("testSingleSubEvent") - l := &testListener{} - cb := callback.New(l) - AddListener(evt, cb) - - Fire(evt, "mySingleSubData") - l.requireEqual(t, 1, "mySingleSubData") - - Fire(evt, "mySingleSubData2") - l.requireEqual(t, 2, "mySingleSubData2") - - RemoveListener(evt, cb) - Fire(evt, "mySingleSubData3") - l.requireEqual(t, 2, "mySingleSubData2") - - AddListener(evt, cb) - Fire(evt, "mySingleSubData4") - l.requireEqual(t, 3, "mySingleSubData4") -} - -func Test_MultipleSubscriptions(t *testing.T) { - evt := EventName("testMultiSubEvent") - l1 := &testListener{} - l2 := &testListener{} - l3 := &testListener{} - cb1 := callback.New(l1) - cb2 := callback.New(l2) - cb3 := callback.New(l3) - - AddListener(evt, cb1) - AddListener(evt, cb2) - AddListener(evt, cb3) - - Fire(evt, "data1") - l1.requireEqual(t, 1, "data1") - l2.requireEqual(t, 1, "data1") - l3.requireEqual(t, 1, "data1") - - Fire(evt, "data2") - l1.requireEqual(t, 2, "data2") - l2.requireEqual(t, 2, "data2") - l3.requireEqual(t, 2, "data2") - - RemoveListener(evt, cb2) - Fire(evt, "data3") - l1.requireEqual(t, 3, "data3") - l2.requireEqual(t, 2, "data2") // Listener 2 removed, should not increment - l3.requireEqual(t, 3, "data3") -} - -func TestMultipleEvents(t *testing.T) { - evt1 := EventName("testMultiEvt1") - evt2 := EventName("testMultiEvt2") - l1 := &testListener{} - l2 := &testListener{} - cb1 := callback.New(l1) - cb2 := callback.New(l2) - - AddListener(evt1, cb1) - AddListener(evt2, cb2) - - Fire(evt1, "data1") - l1.requireEqual(t, 1, "data1") - l2.requireEqual(t, 0, "") - - Fire(evt2, "data2") - l1.requireEqual(t, 1, "data1") - l2.requireEqual(t, 1, "data2") - - Fire(evt1, "data3") - l1.requireEqual(t, 2, "data3") - l2.requireEqual(t, 1, "data2") - - Fire(evt2, "data4") - l1.requireEqual(t, 2, "data3") - l2.requireEqual(t, 2, "data4") -} - -func TestConcurrentEvents(t *testing.T) { - const ( - numEvents = 50 - listenersPerEvent = 20 - invokesPerEvent = 50 - ) - - originalCnt := len(listeners) - evts := make([]EventName, numEvents) - var wg sync.WaitGroup - - // Subscribe to events concurrently - handlers := make([]*testListener, numEvents*listenersPerEvent) - wg.Add(numEvents * listenersPerEvent) - for i := 0; i < numEvents; i++ { - evts[i] = EventName(fmt.Sprintf("testConcurrentEvent-%d", i)) - for j := 0; j < listenersPerEvent; j++ { - go func(i, j int) { - defer wg.Done() - lis := &testListener{} - AddListener(evts[i], callback.New(lis)) - handlers[i*listenersPerEvent+j] = lis - }(i, j) - } - } - wg.Wait() - - // Invoke events concurrently - wg.Add(numEvents * invokesPerEvent) - for i := 0; i < numEvents; i++ { - for j := 0; j < invokesPerEvent; j++ { - go func(i, j int) { - defer wg.Done() - Fire(evts[i], fmt.Sprintf("data-%d-%d", i, j)) - }(i, j) - } - } - wg.Wait() - - // Verify results - require.Len(t, listeners, originalCnt+numEvents) - for i := 0; i < numEvents*listenersPerEvent; i++ { - require.Equal(t, int32(invokesPerEvent), handlers[i].cnt.Load()) - require.Regexp(t, fmt.Sprintf("data-%d-\\d", i/listenersPerEvent), handlers[i].lastData.Load()) - } -} - -type testListener struct { - cnt atomic.Int32 - lastData atomic.Value -} - -func (l *testListener) OnCall(eventData string) string { - l.cnt.Add(1) - l.lastData.Store(eventData) - return "" -} - -func (l *testListener) requireEqual(t *testing.T, cnt int32, data string) { - require.Equal(t, cnt, l.cnt.Load()) - if cnt == 0 { - require.Nil(t, l.lastData.Load()) - } else { - require.Equal(t, data, l.lastData.Load()) - } -} diff --git a/client/go/outline/method_channel.go b/client/go/outline/method_channel.go index 58023e05de8..e5ffe406f2e 100644 --- a/client/go/outline/method_channel.go +++ b/client/go/outline/method_channel.go @@ -22,11 +22,15 @@ import ( // API name constants const ( - // AddEventListener registers a callback for a specific event. + // SetVPNStateChangeListener sets a callback to be invoked when the VPN state changes. // - // - Input: a JSON string of eventListenerJSON + // We recommend the caller to set this listener at app startup to catch all VPN state changes. + // Users might start the VPN from system settings, bypassing the app; + // so setting the listener when connecting within the app might miss some events. + // + // - Input: A callback token string. // - Output: null - MethodAddEventListener = "AddEventListener" + MethodSetVPNStateChangeListener = "SetVPNStateChangeListener" // CloseVPN closes an existing VPN connection and restores network traffic to the default // network interface. @@ -45,12 +49,6 @@ const ( // - Input: the URL string of the resource to fetch // - Output: the content in raw string of the fetched resource MethodFetchResource = "FetchResource" - - // RemoveEventListener unregisters a callback from a specific event. - // - // - Input: a JSON string of eventListenerJSON - // - Output: null - MethodRemoveEventListener = "RemoveEventListener" ) // InvokeMethodResult represents the result of an InvokeMethod call. @@ -64,8 +62,8 @@ type InvokeMethodResult struct { // InvokeMethod calls a method by name. func InvokeMethod(method string, input string) *InvokeMethodResult { switch method { - case MethodAddEventListener: - err := addEventListener(input) + case MethodSetVPNStateChangeListener: + err := setVPNStateChangeListener(input) return &InvokeMethodResult{ Error: platerrors.ToPlatformError(err), } @@ -90,12 +88,6 @@ func InvokeMethod(method string, input string) *InvokeMethodResult { Error: platerrors.ToPlatformError(err), } - case MethodRemoveEventListener: - err := removeEventListener(input) - return &InvokeMethodResult{ - Error: platerrors.ToPlatformError(err), - } - default: return &InvokeMethodResult{Error: &platerrors.PlatformError{ Code: platerrors.InternalError, diff --git a/client/go/outline/vpn/vpn.go b/client/go/outline/vpn/vpn.go index 1dea4befc83..9ee40113dd2 100644 --- a/client/go/outline/vpn/vpn.go +++ b/client/go/outline/vpn/vpn.go @@ -21,7 +21,7 @@ import ( "log/slog" "sync" - "github.com/Jigsaw-Code/outline-apps/client/go/outline/event" + "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" "github.com/Jigsaw-Code/outline-sdk/transport" ) @@ -59,9 +59,6 @@ const ( ConnectionDisconnecting ConnectionStatus = "Disconnecting" ) -// ConnectionStatusChanged event will be raised when the status of a [VPNConnection] changes. -const ConnectionStatusChanged event.EventName = "VPNConnStatusChanged" - // VPNConnection represents a system-wide VPN connection. type VPNConnection struct { ID string `json:"id"` @@ -78,17 +75,25 @@ type VPNConnection struct { // This package allows at most one active VPN connection at the same time. var mu sync.Mutex var conn *VPNConnection +var stateChangeCb callback.Token -// SetStatus sets the [VPNConnection] Status and raises the [ConnectionStatusChanged] event. +// SetStatus sets the [VPNConnection] Status and calls the stateChangeCb callback. func (c *VPNConnection) SetStatus(status ConnectionStatus) { c.Status = status if connJson, err := json.Marshal(c); err == nil { - event.Fire(ConnectionStatusChanged, string(connJson)) + callback.DefaultManager().Call(stateChangeCb, string(connJson)) } else { slog.Warn("failed to marshal VPN connection", "err", err) } } +// SetStateChangeListener sets the given [callback.Token] as a global VPN connection +// state change listener. +// The token should have already been registered with the [callback.DefaultManager]. +func SetStateChangeListener(token callback.Token) { + stateChangeCb = token +} + // EstablishVPN establishes a new active [VPNConnection] connecting to a [ProxyDevice] // with the given VPN [Config]. // It first closes any active [VPNConnection] using [CloseVPN], and then marks the diff --git a/client/go/outline/vpn_linux.go b/client/go/outline/vpn_linux.go index fb2f29b1f34..264349fec26 100644 --- a/client/go/outline/vpn_linux.go +++ b/client/go/outline/vpn_linux.go @@ -17,7 +17,9 @@ package outline import ( "context" "encoding/json" + "strconv" + "github.com/Jigsaw-Code/outline-apps/client/go/outline/callback" perrs "github.com/Jigsaw-Code/outline-apps/client/go/outline/platerrors" "github.com/Jigsaw-Code/outline-apps/client/go/outline/vpn" ) @@ -57,3 +59,16 @@ func establishVPN(configStr string) error { func closeVPN() error { return vpn.CloseVPN() } + +func setVPNStateChangeListener(cbTokenStr string) error { + cbToken, err := strconv.Atoi(cbTokenStr) + if err != nil { + return perrs.PlatformError{ + Code: perrs.InternalError, + Message: "invalid callback token", + Cause: perrs.ToPlatformError(err), + } + } + vpn.SetStateChangeListener(callback.Token(cbToken)) + return nil +} diff --git a/client/go/outline/vpn_others.go b/client/go/outline/vpn_others.go index 7e0c17f4a43..08eddac9d2d 100644 --- a/client/go/outline/vpn_others.go +++ b/client/go/outline/vpn_others.go @@ -18,5 +18,6 @@ package outline import "errors" -func establishVPN(configStr string) error { return errors.ErrUnsupported } -func closeVPN() error { return errors.ErrUnsupported } +func establishVPN(configStr string) error { return errors.ErrUnsupported } +func closeVPN() error { return errors.ErrUnsupported } +func setVPNStateChangeListener(cbTokenStr string) error { return errors.ErrUnsupported } From 535d0c4a9864005f82036e53557e83943db34d5f Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Fri, 31 Jan 2025 00:23:33 -0500 Subject: [PATCH 21/23] fix koffi async call --- client/electron/go_plugin.ts | 4 +-- client/electron/index.ts | 15 +++--------- client/electron/vpn_service.ts | 34 +++++++++++--------------- client/go/outline/callback/callback.go | 4 +-- 4 files changed, 22 insertions(+), 35 deletions(-) diff --git a/client/electron/go_plugin.ts b/client/electron/go_plugin.ts index 7caddd5538f..dafe2ad1f3b 100644 --- a/client/electron/go_plugin.ts +++ b/client/electron/go_plugin.ts @@ -93,7 +93,7 @@ function getDefaultCallbackManager(): CallbackManager { } class CallbackManager { - /** `void (*CallbackFuncPtr)(const char *data);` */ + /** `const char* (*CallbackFuncPtr)(const char *data);` */ private readonly callbackFuncPtr: koffi.IKoffiCType; /** `int RegisterCallback(CallbackFuncPtr cb);` */ @@ -190,6 +190,6 @@ class BackendChannel { result: koffi.TypeSpec, args: koffi.TypeSpec[] ): Function { - return promisify(this.library.func(name, result, args)); + return promisify(this.library.func(name, result, args).async); } } diff --git a/client/electron/index.ts b/client/electron/index.ts index ea538d96504..7420ddba92d 100644 --- a/client/electron/index.ts +++ b/client/electron/index.ts @@ -39,7 +39,7 @@ import {invokeGoMethod} from './go_plugin'; import {GoVpnTunnel} from './go_vpn_tunnel'; import {installRoutingServices, RoutingDaemon} from './routing_service'; import {TunnelStore} from './tunnel_store'; -import {closeVpn, establishVpn, onVpnStatusChanged} from './vpn_service'; +import {closeVpn, establishVpn, onVpnStateChanged} from './vpn_service'; import {VpnTunnel} from './vpn_tunnel'; import * as config from '../src/www/app/outline_server_repository/config'; import { @@ -370,15 +370,6 @@ async function createVpnTunnel( return tunnel; } -/** - * Initializes the VPN service. For example, subscribing to global events. - */ -async function initVpnService() { - if (USE_MODERN_ROUTING) { - onVpnStatusChanged((id, status) => setUiTunnelStatus(status, id)); - } -} - // Invoked by both the start-proxying event handler and auto-connect. async function startVpn(request: StartRequestJson, isAutoConnect: boolean) { console.debug('startVpn called with request ', JSON.stringify(request)); @@ -477,7 +468,9 @@ function main() { // TODO(fortuna): Start the app with the window hidden on auto-start? setupWindow(); - await initVpnService(); + if (USE_MODERN_ROUTING) { + await onVpnStateChanged(setUiTunnelStatus); + } let requestAtShutdown: StartRequestJson | undefined; try { diff --git a/client/electron/vpn_service.ts b/client/electron/vpn_service.ts index 3a6ba52936a..bd26ae63a21 100644 --- a/client/electron/vpn_service.ts +++ b/client/electron/vpn_service.ts @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {invokeGoMethod, newCallback} from './go_plugin'; +import {invokeGoMethod, registerCallback} from './go_plugin'; import { StartRequestJson, TunnelStatus, @@ -73,49 +73,45 @@ export async function closeVpn(): Promise { await invokeGoMethod('CloseVPN', ''); } -export type VpnStatusCallback = (id: string, status: TunnelStatus) => void; +export type VpnStateChangeCallback = (status: TunnelStatus, id: string) => void; /** - * Registers a callback function to be invoked when the VPN status changes. + * Registers a callback function to be invoked when the VPN state changes. * - * @param cb - The callback function to be invoked when the VPN status changes. + * @param cb - The callback function to be invoked when the VPN state changes. * The callback will receive the VPN connection ID as well as the new status. * * @remarks The caller should subscribe to this event **only once**. * Use the `id` parameter in the callback to identify the firing VPN connection. */ -export async function onVpnStatusChanged(cb: VpnStatusCallback): Promise { +export async function onVpnStateChanged( + cb: VpnStateChangeCallback +): Promise { if (!cb) { return; } - const cbToken = await newCallback(data => { + const cbToken = await registerCallback(data => { const conn = JSON.parse(data) as VPNConnectionState; - console.debug(`received ${StatusChangedEvent}`, conn); + console.debug('VPN connection state changed', conn); switch (conn?.status) { case VPNConnConnected: - cb(conn.id, TunnelStatus.CONNECTED); + cb(TunnelStatus.CONNECTED, conn.id); break; case VPNConnConnecting: - cb(conn.id, TunnelStatus.RECONNECTING); + cb(TunnelStatus.RECONNECTING, conn.id); break; case VPNConnDisconnecting: - cb(conn.id, TunnelStatus.DISCONNECTING); + cb(TunnelStatus.DISCONNECTING, conn.id); break; case VPNConnDisconnected: - cb(conn.id, TunnelStatus.DISCONNECTED); + cb(TunnelStatus.DISCONNECTED, conn.id); break; } return ''; }); - await invokeGoMethod( - 'AddEventListener', - JSON.stringify({ - name: StatusChangedEvent, - callbackToken: cbToken, - }) - ); + await invokeGoMethod('SetVPNStateChangeListener', cbToken.toString()); } //#region type definitions of VPNConnection in Go @@ -123,8 +119,6 @@ export async function onVpnStatusChanged(cb: VpnStatusCallback): Promise { // The following constants and types should be aligned with the corresponding definitions // in `./client/go/outline/vpn/vpn.go`. -const StatusChangedEvent = 'VPNConnStatusChanged'; - type VPNConnStatus = string; const VPNConnConnecting: VPNConnStatus = 'Connecting'; const VPNConnConnected: VPNConnStatus = 'Connected'; diff --git a/client/go/outline/callback/callback.go b/client/go/outline/callback/callback.go index ecc16b5ac1d..8010a7fa9df 100644 --- a/client/go/outline/callback/callback.go +++ b/client/go/outline/callback/callback.go @@ -73,7 +73,7 @@ func (m *Manager) Register(c Handler) Token { token := m.nextCbID m.nextCbID++ m.callbacks[token] = c - slog.Debug("callback created", "manager", m, "token", token) + slog.Debug("callback created", "token", token) return token } @@ -85,7 +85,7 @@ func (m *Manager) Unregister(token Token) { defer m.mu.Unlock() delete(m.callbacks, token) - slog.Debug("callback deleted", "manager", m, "token", token) + slog.Debug("callback deleted", "token", token) } // Call invokes the callback identified by the given [Token]. From 75d5a45003140c92740c2103fdfb23f5640e6e94 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Fri, 31 Jan 2025 00:41:30 -0500 Subject: [PATCH 22/23] resolve conflicts --- client/go/outline/method_channel.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/client/go/outline/method_channel.go b/client/go/outline/method_channel.go index dd563609c7d..8cf1b24b798 100644 --- a/client/go/outline/method_channel.go +++ b/client/go/outline/method_channel.go @@ -31,9 +31,9 @@ const ( // EstablishVPN initiates a VPN connection and directs all network traffic through Outline. // - // - Input: A callback token string. - // - Output: null - MethodSetVPNStateChangeListener = "SetVPNStateChangeListener" + // - Input: a JSON string of vpn.configJSON. + // - Output: a JSON string of vpn.connectionJSON. + MethodEstablishVPN = "EstablishVPN" // FetchResource fetches a resource located at a given URL. // - Input: the URL string of the resource to fetch @@ -44,6 +44,16 @@ const ( // - Input: the transport config text // - Output: the TunnelConfigJson that Typescript needs MethodParseTunnelConfig = "ParseTunnelConfig" + + // SetVPNStateChangeListener sets a callback to be invoked when the VPN state changes. + // + // We recommend the caller to set this listener at app startup to catch all VPN state changes. + // Users might start the VPN from system settings, bypassing the app; + // so setting the listener when connecting within the app might miss some events. + // + // - Input: A callback token string. + // - Output: null + MethodSetVPNStateChangeListener = "SetVPNStateChangeListener" ) // InvokeMethodResult represents the result of an InvokeMethod call. @@ -80,6 +90,12 @@ func InvokeMethod(method string, input string) *InvokeMethodResult { case MethodParseTunnelConfig: return doParseTunnelConfig(input) + case MethodSetVPNStateChangeListener: + err := setVPNStateChangeListener(input) + return &InvokeMethodResult{ + Error: platerrors.ToPlatformError(err), + } + default: return &InvokeMethodResult{Error: &platerrors.PlatformError{ Code: platerrors.InternalError, From ca2b263ce10ee7f7256a1239c81fbc970bd6ac99 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Fri, 31 Jan 2025 00:58:43 -0500 Subject: [PATCH 23/23] update comment --- client/electron/go_plugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/electron/go_plugin.ts b/client/electron/go_plugin.ts index dafe2ad1f3b..c632182895c 100644 --- a/client/electron/go_plugin.ts +++ b/client/electron/go_plugin.ts @@ -82,7 +82,7 @@ export async function unregisterCallback(token: CallbackToken): Promise { console.debug('[Backend] - unregisterCallback done, token:', token); } -/** Singleton of the CGo backend channel. */ +/** Singleton of the CGo callback manager. */ let callback: CallbackManager | undefined; function getDefaultCallbackManager(): CallbackManager {