Improve gui app disposal handling and close all network connections

app/gui2/shared/dataServer.ts

@@ -29,6 +29,7 @@ import {
   type Offset,
   type Table,
 } from './binaryProtocol'
+import type { AbortScope } from './util/net'
 import { uuidFromBits, uuidToBits } from './uuid'
 import type { WebsocketClient } from './websocket'
 import type { Uuid } from './yjsModel'
@@ -58,8 +59,12 @@ export class DataServer extends ObservableV2<DataServerEvents> {
   resolveCallbacks = new Map<string, (data: any) => void>()
 
   /** `websocket.binaryType` should be `ArrayBuffer`. */
-  constructor(public websocket: WebsocketClient) {
+  constructor(
+    public websocket: WebsocketClient,
+    abort: AbortScope,
+  ) {
     super()
+    abort.handleDispose(this)
     if (websocket.connected) {
       this.ready = Promise.resolve()
     } else {
@@ -95,6 +100,10 @@ export class DataServer extends ObservableV2<DataServerEvents> {
     })
   }
 
+  dispose() {
+    this.resolveCallbacks.clear()
+  }
+
   async initialize(clientId: Uuid) {
     if (!this.initialized) {
       this.clientId = clientId

app/gui2/shared/languageServer.ts

@@ -21,6 +21,7 @@ import type {
   VisualizationConfiguration,
   response,
 } from './languageServerTypes'
+import type { AbortScope } from './util/net'
 import type { Uuid } from './yjsModel'
 
 const DEBUG_LOG_RPC = false
@@ -107,6 +108,7 @@ export class LsRpcError extends Error {
 export class LanguageServer extends ObservableV2<Notifications> {
   client: Client
   handlers: Map<string, Set<(...params: any[]) => void>>
+  retainCount = 1
 
   constructor(client: Client) {
     super()
@@ -124,6 +126,7 @@ export class LanguageServer extends ObservableV2<Notifications> {
   // The "magic bag of holding" generic that is only present in the return type is UNSOUND.
   // However, it is SAFE, as the return type of the API is statically known.
   private async request<T>(method: string, params: object): Promise<T> {
+    if (this.retainCount === 0) return Promise.reject(new Error('LanguageServer disposed'))
     const uuid = uuidv4()
     const now = performance.now()
     try {
@@ -432,8 +435,22 @@ export class LanguageServer extends ObservableV2<Notifications> {
     }
   }
 
-  dispose() {
-    this.client.close()
+  retain() {
+    if (this.retainCount === 0) {
+      throw new Error('Trying to retain already disposed language server.')
+    }
+    this.retainCount += 1
+  }
+
+  release() {
+    if (this.retainCount > 0) {
+      this.retainCount -= 1
+      if (this.retainCount === 0) {
+        this.client.close()
+      }
+    } else {
+      throw new Error('Released already disposed language server.')
+    }
   }
 }
 

app/gui2/shared/util/net.ts

@@ -0,0 +1,39 @@
+import type { ObservableV2 } from 'lib0/observable'
+
+interface Disposable {
+  dispose(): void
+}
+
+export class AbortScope {
+  private ctrl: AbortController = new AbortController()
+  get signal() {
+    return this.ctrl.signal
+  }
+
+  dispose(reason?: string) {
+    this.ctrl.abort(reason)
+  }
+
+  handleDispose(disposable: Disposable) {
+    this.signal.throwIfAborted()
+    this.onAbort(disposable.dispose.bind(disposable))
+  }
+
+  onAbort(listener: () => void) {
+    if (this.signal.aborted) {
+      setTimeout(listener, 0)
+    } else {
+      this.signal.addEventListener('abort', listener, { once: true })
+    }
+  }
+
+  handleObserve<
+    EVENTS extends { [key in keyof EVENTS]: (...arg0: any[]) => void },
+    NAME extends keyof EVENTS & string,
+  >(observable: ObservableV2<EVENTS>, name: NAME, f: EVENTS[NAME]) {
+    if (this.signal.aborted) return
+    observable.on(name, f)
+    this.onAbort(() => observable.off(name, f))
+    return f
+  }
+}

app/gui2/shared/websocket.ts

@@ -38,6 +38,7 @@
 import * as math from 'lib0/math'
 import { ObservableV2 } from 'lib0/observable'
 import * as time from 'lib0/time'
+import type { AbortScope } from './util/net'
 
 const reconnectTimeoutBase = 1200
 const maxReconnectTimeout = 2500
@@ -130,12 +131,14 @@ export class WebsocketClient extends ObservableV2<WebsocketEvents> {
   protected _checkInterval
   constructor(
     public url: string,
+    abort: AbortScope,
     {
       binaryType,
       sendPings,
     }: { binaryType?: 'arraybuffer' | 'blob' | null; sendPings?: boolean } = {},
   ) {
     super()
+    abort.handleDispose(this)
     this.ws = null
     this.binaryType = binaryType || null
     this.sendPings = sendPings ?? true
@@ -168,7 +171,7 @@ export class WebsocketClient extends ObservableV2<WebsocketEvents> {
     this.ws.send(encoded)
   }
 
-  destroy() {
+  dispose() {
     clearInterval(this._checkInterval)
     this.disconnect()
     super.destroy()

app/gui2/src/appRunner.ts

@@ -24,15 +24,14 @@ async function runApp(
   // until GUI1 is removed, as GUI1 still needs them.
   const intermediateConfig = mergeConfig(baseConfig, urlParams())
   const appConfig = mergeConfig(intermediateConfig, config ?? {})
-  const app = await mountProjectApp(
+  unmount = await mountProjectApp(
     {
       config: appConfig,
       accessToken,
       unrecognizedOptions,
     },
     pinia,
   )
-  unmount = () => app.unmount()
 }
 
 function stopApp() {

app/gui2/src/components/GraphEditor/GraphNode.vue

@@ -28,7 +28,8 @@ import { Vec2 } from '@/util/data/vec2'
 import { displayedIconOf } from '@/util/getIconName'
 import { setIfUndefined } from 'lib0/map'
 import type { VisualizationIdentifier } from 'shared/yjsModel'
-import { computed, onUnmounted, ref, watch, watchEffect } from 'vue'
+import type { EffectScope } from 'vue'
+import { computed, effectScope, onScopeDispose, onUnmounted, ref, watch, watchEffect } from 'vue'
 
 const MAXIMUM_CLICK_LENGTH_MS = 300
 const MAXIMUM_CLICK_DISTANCE_SQ = 50
@@ -347,25 +348,38 @@ const outputPorts = computed((): PortData[] => {
 })
 
 const outputHovered = ref<AstId>()
-const hoverAnimations = new Map<AstId, ReturnType<typeof useApproach>>()
+const hoverAnimations = new Map<AstId, [ReturnType<typeof useApproach>, EffectScope]>()
 watchEffect(() => {
   const ports = outputPortsSet.value
-  for (const key of hoverAnimations.keys()) if (!ports.has(key)) hoverAnimations.delete(key)
+  for (const key of hoverAnimations.keys())
+    if (!ports.has(key)) {
+      hoverAnimations.get(key)?.[1].stop()
+      hoverAnimations.delete(key)
+    }
   for (const port of outputPortsSet.value) {
-    setIfUndefined(hoverAnimations, port, () =>
-      useApproach(
-        () => (outputHovered.value === port || graph.unconnectedEdge?.target === port ? 1 : 0),
-        50,
-        0.01,
-      ),
-    )
+    setIfUndefined(hoverAnimations, port, () => {
+      // Because `useApproach` uses `onScopeDispose` and we are calling it dynamically (i.e. not at
+      // the setup top-level), we need to create a detached scope for each invocation.
+      const scope = effectScope(true)
+      const approach = scope.run(() =>
+        useApproach(
+          () => (outputHovered.value === port || graph.unconnectedEdge?.target === port ? 1 : 0),
+          50,
+          0.01,
+        ),
+      )!
+      return [approach, scope]
+    })
   }
 })
 
+// Clean up dynamically created detached scopes.
+onScopeDispose(() => hoverAnimations.forEach(([_, scope]) => scope.stop()))
+
 function portGroupStyle(port: PortData) {
   const [start, end] = port.clipRange
   return {
-    '--hover-animation': hoverAnimations.get(port.portId)?.value ?? 0,
+    '--hover-animation': hoverAnimations.get(port.portId)?.[0].value ?? 0,
     '--port-clip-start': start,
     '--port-clip-end': end,
   }

app/gui2/src/createApp.ts

@@ -19,8 +19,26 @@ export async function mountProjectApp(
   await initializeFFI()
   initializePrefixes()
 
+  const usedPinia = pinia ?? createPinia()
   const app = createApp(App, rootProps)
-  app.use(pinia ?? createPinia())
+  app.use(usedPinia)
   app.mount('#app')
-  return app
+  return () => {
+    app.unmount()
+    console.log('app unmounted')
+    disposePinia(usedPinia)
+  }
+}
+
+// Hack: `disposePinia` is not yet officially released, but we desperately need this for correct app
+// cleanup. Pasted code from git version seems to work fine. This should be replaced with pinia
+// export once it is available. Code copied from:
+// https://github.com/vuejs/pinia/blob/8835e98173d9443531a7d65dfed09c2a8c19975d/packages/pinia/src/createPinia.ts#L74
+export function disposePinia(pinia: Pinia) {
+  const anyPinia: any = pinia
+  anyPinia._e.stop()
+  anyPinia._s.clear()
+  anyPinia._p.splice(0)
+  anyPinia.state.value = {}
+  anyPinia._a = null
 }

app/gui2/src/stores/project/computedValueRegistry.ts

@@ -53,7 +53,7 @@ export class ComputedValueRegistry {
     return this.db.get(exprId)
   }
 
-  destroy() {
+  dispose() {
     this.executionContext?.off('expressionUpdates', this._updateHandler)
   }
 }