Browse Source

fix: Explicitly exit CLI to prevent hanging subprocesses (#3083)

Co-authored-by: Aiden Cline <[email protected]>
Haris Gušić 4 months ago
parent
commit
a9624c0fff

+ 2 - 1
packages/opencode/src/file/watcher.ts

@@ -63,7 +63,8 @@ export namespace FileWatcher {
       return { sub }
       return { sub }
     },
     },
     async (state) => {
     async (state) => {
-      state.sub?.unsubscribe()
+      if (!state.sub) return
+      await state.sub?.unsubscribe()
     },
     },
   )
   )
 
 

+ 6 - 4
packages/opencode/src/index.ts

@@ -22,8 +22,6 @@ import { AttachCommand } from "./cli/cmd/attach"
 import { AcpCommand } from "./cli/cmd/acp"
 import { AcpCommand } from "./cli/cmd/acp"
 import { EOL } from "os"
 import { EOL } from "os"
 
 
-const cancel = new AbortController()
-
 process.on("unhandledRejection", (e) => {
 process.on("unhandledRejection", (e) => {
   Log.Default.error("rejection", {
   Log.Default.error("rejection", {
     e: e instanceof Error ? e.message : e,
     e: e instanceof Error ? e.message : e,
@@ -135,6 +133,10 @@ try {
     console.error(e)
     console.error(e)
   }
   }
   process.exitCode = 1
   process.exitCode = 1
+} finally {
+  // Some subprocesses don't react properly to SIGTERM and similar signals.
+  // Most notably, some docker-container-based MCP servers don't handle such signals unless
+  // run using `docker run --init`.
+  // Explicitly exit to avoid any hanging subprocesses.
+  process.exit();
 }
 }
-
-cancel.abort()

+ 1 - 3
packages/opencode/src/lsp/index.ts

@@ -101,9 +101,7 @@ export namespace LSP {
       }
       }
     },
     },
     async (state) => {
     async (state) => {
-      for (const client of state.clients) {
-        await client.shutdown()
-      }
+      await Promise.all(state.clients.map((client) => client.shutdown()))
     },
     },
   )
   )
 
 

+ 1 - 3
packages/opencode/src/mcp/index.ts

@@ -145,9 +145,7 @@ export namespace MCP {
       }
       }
     },
     },
     async (state) => {
     async (state) => {
-      for (const client of Object.values(state.clients)) {
-        client.close()
-      }
+      await Promise.all(Object.values(state.clients).map((client) => client.close()))
     },
     },
   )
   )
 
 

+ 1 - 1
packages/opencode/src/project/bootstrap.ts

@@ -11,7 +11,7 @@ export async function InstanceBootstrap() {
   await Plugin.init()
   await Plugin.init()
   Share.init()
   Share.init()
   Format.init()
   Format.init()
-  LSP.init()
+  await LSP.init()
   FileWatcher.init()
   FileWatcher.init()
   File.init()
   File.init()
 }
 }

+ 41 - 9
packages/opencode/src/project/state.ts

@@ -1,23 +1,26 @@
+import { Log } from "@/util/log"
+
 export namespace State {
 export namespace State {
   interface Entry {
   interface Entry {
     state: any
     state: any
     dispose?: (state: any) => Promise<void>
     dispose?: (state: any) => Promise<void>
   }
   }
 
 
-  const entries = new Map<string, Map<any, Entry>>()
+  const log = Log.create({ service: "state" })
+  const recordsByKey = new Map<string, Map<any, Entry>>()
 
 
   export function create<S>(root: () => string, init: () => S, dispose?: (state: Awaited<S>) => Promise<void>) {
   export function create<S>(root: () => string, init: () => S, dispose?: (state: Awaited<S>) => Promise<void>) {
     return () => {
     return () => {
       const key = root()
       const key = root()
-      let collection = entries.get(key)
-      if (!collection) {
-        collection = new Map<string, Entry>()
-        entries.set(key, collection)
+      let entries = recordsByKey.get(key)
+      if (!entries) {
+        entries = new Map<string, Entry>()
+        recordsByKey.set(key, entries)
       }
       }
-      const exists = collection.get(init)
+      const exists = entries.get(init)
       if (exists) return exists.state as S
       if (exists) return exists.state as S
       const state = init()
       const state = init()
-      collection.set(init, {
+      entries.set(init, {
         state,
         state,
         dispose,
         dispose,
       })
       })
@@ -26,9 +29,38 @@ export namespace State {
   }
   }
 
 
   export async function dispose(key: string) {
   export async function dispose(key: string) {
-    for (const [_, entry] of entries.get(key)?.entries() ?? []) {
+    const entries = recordsByKey.get(key)
+    if (!entries) return
+
+    log.info("waiting for state disposal to complete", { key })
+
+    let disposalFinished = false
+
+    setTimeout(() => {
+      if (!disposalFinished) {
+        log.warn(
+          "state disposal is taking an unusually long time - if it does not complete in a reasonable time, please report this as a bug",
+          { key },
+        )
+      }
+    }, 10000).unref()
+
+    const tasks: Promise<void>[] = []
+    for (const entry of entries.values()) {
       if (!entry.dispose) continue
       if (!entry.dispose) continue
-      await entry.dispose(await entry.state)
+
+      const task = Promise.resolve(entry.state)
+        .then((state) => entry.dispose!(state))
+        .catch((error) => {
+          log.error("Error while disposing state:", { error, key })
+        })
+
+      tasks.push(task)
     }
     }
+
+    await Promise.all(tasks)
+
+    disposalFinished = true
+    log.info("state disposal completed", { key })
   }
   }
 }
 }

+ 19 - 30
packages/opencode/src/session/prompt.ts

@@ -74,13 +74,22 @@ export namespace SessionPrompt {
           callback: (input: MessageV2.WithParts) => void
           callback: (input: MessageV2.WithParts) => void
         }[]
         }[]
       >()
       >()
+      const pending = new Set<Promise<void>>()
+
+      const track = (promise: Promise<void>) => {
+        pending.add(promise)
+        promise.finally(() => pending.delete(promise))
+      }
 
 
       return {
       return {
         queued,
         queued,
+        pending,
+        track,
       }
       }
     },
     },
     async (current) => {
     async (current) => {
       current.queued.clear()
       current.queued.clear()
+      await Promise.allSettled([...current.pending])
     },
     },
   )
   )
 
 
@@ -191,28 +200,6 @@ export namespace SessionPrompt {
       processor,
       processor,
     })
     })
 
 
-    // const permUnsub = (() => {
-    //   const handled = new Set<string>()
-    //   const options = [
-    //     { optionId: "allow_once", kind: "allow_once", name: "Allow once" },
-    //     { optionId: "allow_always", kind: "allow_always", name: "Always allow" },
-    //     { optionId: "reject_once", kind: "reject_once", name: "Reject" },
-    //   ]
-    //   return Bus.subscribe(Permission.Event.Updated, async (event) => {
-    //     const info = event.properties
-    //     if (info.sessionID !== input.sessionID) return
-    //     if (handled.has(info.id)) return
-    //     handled.add(info.id)
-    //     const toolCallId = info.callID ?? info.id
-    //     const metadata = info.metadata ?? {}
-    //     // TODO: emit permission event to bus for ACP to handle
-    //     Permission.respond({ sessionID: info.sessionID, permissionID: info.id, response: "reject" })
-    //   })
-    // })()
-    // await using _permSub = defer(() => {
-    //   permUnsub?.()
-    // })
-
     const params = await Plugin.trigger(
     const params = await Plugin.trigger(
       "chat.params",
       "chat.params",
       {
       {
@@ -247,13 +234,15 @@ export namespace SessionPrompt {
       step++
       step++
       await processor.next(msgs.findLast((m) => m.info.role === "user")?.info.id!)
       await processor.next(msgs.findLast((m) => m.info.role === "user")?.info.id!)
       if (step === 1) {
       if (step === 1) {
-        ensureTitle({
-          session,
-          history: msgs,
-          message: userMsg,
-          providerID: model.providerID,
-          modelID: model.info.id,
-        })
+        state().track(
+          ensureTitle({
+            session,
+            history: msgs,
+            message: userMsg,
+            providerID: model.providerID,
+            modelID: model.info.id,
+          }),
+        )
         SessionSummary.summarize({
         SessionSummary.summarize({
           sessionID: input.sessionID,
           sessionID: input.sessionID,
           messageID: userMsg.info.id,
           messageID: userMsg.info.id,
@@ -1730,7 +1719,7 @@ export namespace SessionPrompt {
         thinkingBudget: 0,
         thinkingBudget: 0,
       }
       }
     }
     }
-    generateText({
+    await generateText({
       maxOutputTokens: small.info.reasoning ? 1500 : 20,
       maxOutputTokens: small.info.reasoning ? 1500 : 20,
       providerOptions: ProviderTransform.providerOptions(small.npm, small.providerID, options),
       providerOptions: ProviderTransform.providerOptions(small.npm, small.providerID, options),
       messages: [
       messages: [