Fix stuck container remove with timed raw Docker stop/kill/delete.

server/handlers/containers.js

@@ -2,7 +2,14 @@
  * Container RPC handlers.
  */
 import { PassThrough } from 'stream'
-import { docker, startContainerNoBody, containerLifecycleNoBody } from '../services/docker.js'
+import {
+  docker,
+  startContainerNoBody,
+  containerLifecycleNoBody,
+  stopContainerNoBody,
+  killContainerNoBody,
+  removeContainerNoBody,
+} from '../services/docker.js'
 import * as validation from '../utils/validation.js'
 import { Pushes } from '../../shared/protocol.js'
 import { peers } from '../core/peer-registry.js'
@@ -28,7 +35,7 @@ function isNoSuchContainer(err) {
 
 /**
  * Release attachments that can delay Docker remove, stop the container, then delete.
- * Always stops first so processes shut down cleanly before removal.
+ * Uses raw Engine socket calls with hard deadlines so remove never hangs on dockerode.
  * @param {string} id
  * @param {import('../rpc/session.js').PeerSession} session
  * @param {{ force?: boolean, v?: boolean, removeVolumes?: boolean, link?: boolean, timeout?: number, t?: number }} args
@@ -42,59 +49,73 @@ async function forceRemoveContainer(id, session, args = {}) {
     // ignore
   }
 
-  const container = docker.getContainer(id)
   const force = args.force !== false
-  const removeOpts = {
-    force,
-    v: Boolean(args.v || args.removeVolumes),
-  }
-  if (args.link) removeOpts.link = true
+  const removeVolumes = Boolean(args.v || args.removeVolumes)
+  const shortId = String(id || '').slice(0, 12)
 
-  // Stop first so the container shuts down cleanly before remove
+  // Graceful stop first (short grace), then kill fallback, then force-remove
   const stopTimeout =
     Number(args.timeout) >= 0
       ? Number(args.timeout)
       : Number(args.t) >= 0
         ? Number(args.t)
-        : 10
+        : 5
+
   try {
-    await container.stop({ t: stopTimeout })
+    await stopContainerNoBody(id, { t: stopTimeout })
   } catch (err) {
-    // not running / already dead — fine
-    if (!isNoSuchContainer(err) && !/is not running|already stopped/i.test(String(err?.message || ''))) {
-      logger.debug('pre-remove stop skipped', { id: id.slice(0, 12), error: err?.message })
-      // If force and stop failed unexpectedly, fall back to SIGKILL so remove can proceed
-      if (force) {
-        try {
-          await container.kill({ signal: 'SIGKILL' })
-        } catch (killErr) {
-          if (
-            !isNoSuchContainer(killErr) &&
-            !/is not running|already stopped/i.test(String(killErr?.message || ''))
-          ) {
-            logger.debug('pre-remove kill skipped', {
-              id: id.slice(0, 12),
-              error: killErr?.message,
-            })
-          }
+    if (isNoSuchContainer(err)) return
+    logger.debug('pre-remove stop failed', { id: shortId, error: err?.message })
+    if (force) {
+      try {
+        await killContainerNoBody(id, { signal: 'SIGKILL' })
+      } catch (killErr) {
+        if (!isNoSuchContainer(killErr)) {
+          logger.debug('pre-remove kill failed', {
+            id: shortId,
+            error: killErr?.message,
+          })
         }
       }
     }
   }
 
   try {
-    await container.remove(removeOpts)
+    await removeContainerNoBody(id, {
+      force,
+      v: removeVolumes,
+      link: Boolean(args.link),
+    })
   } catch (err) {
     if (isNoSuchContainer(err)) return
     // Concurrent remove in progress — wait briefly, then treat as gone if inspect 404s
     if (/already in progress|removal of container/i.test(String(err?.message || ''))) {
       await new Promise((r) => setTimeout(r, 750))
       try {
-        await container.inspect()
+        await docker.getContainer(id).inspect()
       } catch (inspectErr) {
         if (isNoSuchContainer(inspectErr)) return
       }
     }
+    // Last resort: dockerode force-remove with a hard deadline (never hang the RPC)
+    if (force) {
+      try {
+        await Promise.race([
+          docker.getContainer(id).remove({
+            force: true,
+            v: removeVolumes,
+            ...(args.link ? { link: true } : {}),
+          }),
+          new Promise((_, reject) =>
+            setTimeout(() => reject(new Error('dockerode remove timed out')), 20_000)
+          ),
+        ])
+        return
+      } catch (rmErr) {
+        if (isNoSuchContainer(rmErr)) return
+        throw rmErr
+      }
+    }
     throw err
   }
 }

server/services/docker.js

@@ -56,23 +56,31 @@ function normalizeContainerRef(id) {
 }
 
 /**
- * POST to Docker Engine over the unix socket with a guaranteed-empty body.
+ * HTTP request to Docker Engine over the unix socket with a guaranteed-empty body.
  *
  * Why not dockerode `.start()` / modem.dial?
  * - dockerode always passes `options: {}` into modem for start
  * - bare-http / docker-modem can still emit a JSON body or Content-Type that
  *   modern Engine (API ≥1.24) rejects:
  *   "starting container with non-empty request body was deprecated..."
+ * - dockerode stop/remove can hang indefinitely under busy/hijacked streams;
+ *   raw socket requests always have a hard deadline.
  *
  * We write the HTTP request by hand: Content-Length: 0, no payload bytes.
  *
+ * @param {string} method HTTP method
  * @param {string} apiPath e.g. /containers/abc/start (leading slash, no host)
- * @param {{ ok?: number[] }} [opts]
+ * @param {{ ok?: number[], timeoutMs?: number }} [opts]
  * @returns {Promise<{ statusCode: number, body: string }>}
  */
-function dockerUnixPostEmpty(apiPath, opts = {}) {
+function dockerUnixEmpty(method, apiPath, opts = {}) {
   const okCodes = opts.ok || [204, 304]
+  const timeoutMs =
+    Number(opts.timeoutMs) > 0 && Number.isFinite(Number(opts.timeoutMs))
+      ? Number(opts.timeoutMs)
+      : 60_000
   const path = apiPath.startsWith('/') ? apiPath : `/${apiPath}`
+  const verb = String(method || 'POST').toUpperCase()
 
   return new Promise((resolve, reject) => {
     const socket = net.createConnection({ path: socketPath })
@@ -92,13 +100,13 @@ function dockerUnixPostEmpty(apiPath, opts = {}) {
     }
 
     const timer = setTimeout(() => {
-      finish(new Error(`Docker socket timeout (${path})`))
-    }, 60_000)
+      finish(new Error(`Docker socket timeout (${verb} ${path})`))
+    }, timeoutMs)
 
     socket.on('connect', () => {
-      // Minimal HTTP/1.1 POST — zero-length body, no Content-Type
+      // Minimal HTTP/1.1 — zero-length body, no Content-Type
       const req =
-        `POST ${path} HTTP/1.1\r\n` +
+        `${verb} ${path} HTTP/1.1\r\n` +
         `Host: localhost\r\n` +
         `Content-Length: 0\r\n` +
         `Connection: close\r\n` +
@@ -143,6 +151,16 @@ function dockerUnixPostEmpty(apiPath, opts = {}) {
   })
 }
 
+/**
+ * POST to Docker Engine over the unix socket with a guaranteed-empty body.
+ * @param {string} apiPath e.g. /containers/abc/start (leading slash, no host)
+ * @param {{ ok?: number[], timeoutMs?: number }} [opts]
+ * @returns {Promise<{ statusCode: number, body: string }>}
+ */
+function dockerUnixPostEmpty(apiPath, opts = {}) {
+  return dockerUnixEmpty('POST', apiPath, opts)
+}
+
 /**
  * Start a container with an empty POST body (Engine API ≥1.24).
  * @param {string} id container id or name
@@ -162,6 +180,95 @@ export async function startContainerNoBody(id) {
   }
 }
 
+/**
+ * Graceful stop via raw socket (query `t` = seconds before SIGKILL).
+ * Treats already-stopped as success.
+ * @param {string} id
+ * @param {{ t?: number, timeoutMs?: number }} [opts]
+ */
+export async function stopContainerNoBody(id, opts = {}) {
+  const cid = normalizeContainerRef(id)
+  const t = Number(opts.t)
+  const grace = Number.isFinite(t) && t >= 0 ? Math.min(Math.floor(t), 600) : 5
+  // Allow grace period + small buffer; never hang forever
+  const timeoutMs =
+    Number(opts.timeoutMs) > 0
+      ? Number(opts.timeoutMs)
+      : Math.max(15_000, (grace + 8) * 1000)
+  try {
+    await dockerUnixPostEmpty(`/containers/${cid}/stop?t=${grace}`, {
+      ok: [204, 304],
+      timeoutMs,
+    })
+  } catch (err) {
+    if (err?.statusCode === 304 || /is not running|already stopped/i.test(String(err?.message || ''))) {
+      return
+    }
+    if (err?.statusCode === 404 || /no such container/i.test(String(err?.message || ''))) {
+      const e = new Error(err.message || 'no such container')
+      e.statusCode = 404
+      throw e
+    }
+    throw err
+  }
+}
+
+/**
+ * SIGKILL via raw socket. Treats not-running as success.
+ * @param {string} id
+ * @param {{ signal?: string, timeoutMs?: number }} [opts]
+ */
+export async function killContainerNoBody(id, opts = {}) {
+  const cid = normalizeContainerRef(id)
+  const signal = encodeURIComponent(String(opts.signal || 'SIGKILL'))
+  const timeoutMs = Number(opts.timeoutMs) > 0 ? Number(opts.timeoutMs) : 10_000
+  try {
+    await dockerUnixPostEmpty(`/containers/${cid}/kill?signal=${signal}`, {
+      // 409 = container is not running
+      ok: [204, 409],
+      timeoutMs,
+    })
+  } catch (err) {
+    if (
+      err?.statusCode === 409 ||
+      err?.statusCode === 404 ||
+      /is not running|already stopped|no such container/i.test(String(err?.message || ''))
+    ) {
+      return
+    }
+    throw err
+  }
+}
+
+/**
+ * Force-remove container via raw DELETE (avoids dockerode hang on busy containers).
+ * @param {string} id
+ * @param {{ force?: boolean, v?: boolean, link?: boolean, timeoutMs?: number }} [opts]
+ */
+export async function removeContainerNoBody(id, opts = {}) {
+  const cid = normalizeContainerRef(id)
+  const force = opts.force !== false
+  const v = Boolean(opts.v)
+  const link = Boolean(opts.link)
+  const parts = []
+  if (force) parts.push('force=1')
+  if (v) parts.push('v=1')
+  if (link) parts.push('link=1')
+  const path = `/containers/${cid}${parts.length ? `?${parts.join('&')}` : ''}`
+  const timeoutMs = Number(opts.timeoutMs) > 0 ? Number(opts.timeoutMs) : 30_000
+  try {
+    await dockerUnixEmpty('DELETE', path, {
+      ok: [204, 404],
+      timeoutMs,
+    })
+  } catch (err) {
+    if (err?.statusCode === 404 || /no such container/i.test(String(err?.message || ''))) {
+      return
+    }
+    throw err
+  }
+}
+
 /**
  * Body-less container lifecycle POSTs (pause / unpause).
  * @param {string} id