From 84b8fcdfea32167f260a8869f6ef401bd8fb5b7e Mon Sep 17 00:00:00 2001 From: Nowaker Date: Wed, 9 Sep 2026 22:47:52 -0500 Subject: [PATCH] fix(core): keep a refused inotify instance from wedging the process @parcel/watcher builds its shared inotify backend inside the synchronous constructor of the N-API SubscribeRunner, which runs on whichever thread called subscribe() - for opencode, the JS main thread. When inotify_init1 fails there, InotifyBackend::start() throws before notifyStarted(), and Backend::handleError() only notifies watchers that are already registered. The watcher for this call is not registered until execute() runs, so nothing ever notifies mStartedSignal and Backend::run() waits on it forever. That parks the event loop, not just one fiber. It is why the existing Effect.timeout(SUBSCRIBE_TIMEOUT_MS) and Effect.catchCause around the subscription never fired, why nothing reached the log, and why the turn could not be interrupted: no timer, fiber or signal handler runs again once the main thread stops returning to the loop. Reaching that state needs no bug of ours. fs.inotify.max_user_instances is a per-uid ceiling shared with every other process the user runs, it defaults to 1024, and it cannot be raised without root. Where this was found, 954 instances were held by an unrelated herd of orphaned kbuildsycoca6 processes; every opencode turn afterwards hung on an assistant row with no parts, no error and no completion time. So ask the kernel for one instance first, give it straight back, and call subscribe() only if that succeeded. The probe and the call sit in one synchronous thunk so nothing runs between them, which keeps the window where another process could take the last instance as narrow as it can be made from this side. It cannot be closed entirely: only fixing @parcel/watcher, or owning the watcher in a killable subprocess, removes the race outright. What it does remove is the permanent silent wedge - a refused instance now logs a warning naming the errno, and the turn carries on without file watching. EMFILE does not identify which ceiling was hit, the per-process descriptor limit or the per-uid inotify one, so the raw errno is logged without interpreting it. The probe needs bun:ffi, so it loads only on the inotify branch and only when that import succeeds. A runtime without it keeps the existing behaviour rather than silently losing file watching. AI-Tool: opencode AI-Model: anthropic/claude-opus-5 AI-Platform: darwin AI-Harness: Vibeterm 123ec5e1-dirty AI-Session-ID: ses_f76ce0cf6ffejNGTCJyk7ooYc5 --- .../core/src/filesystem/watcher-inotify.ts | 76 +++++++ packages/core/src/filesystem/watcher.ts | 208 +++++++++++------- .../test/filesystem/watcher-preflight.test.ts | 124 +++++++++++ 3 files changed, 330 insertions(+), 78 deletions(-) create mode 100644 packages/core/src/filesystem/watcher-inotify.ts create mode 100644 packages/core/test/filesystem/watcher-preflight.test.ts diff --git a/packages/core/src/filesystem/watcher-inotify.ts b/packages/core/src/filesystem/watcher-inotify.ts new file mode 100644 index 000000000000..dac33535695b --- /dev/null +++ b/packages/core/src/filesystem/watcher-inotify.ts @@ -0,0 +1,76 @@ +export * as WatcherInotify from "./watcher-inotify" + +import { dlopen, FFIType, read } from "bun:ffi" +import os from "os" +import { lazy } from "../util/lazy" + +declare const OPENCODE_LIBC: string | undefined + +// The flags @parcel/watcher's InotifyBackend passes to inotify_init1. Probing +// with any other flags would not prove that the real call can succeed. +const IN_NONBLOCK = 0o4000 +const IN_CLOEXEC = 0o2000000 + +export interface ProbeFailure { + readonly errno: number + readonly code: string +} + +const errnoCodes = Object.entries(os.constants.errno) + +const libc = lazy(() => { + const musl = `libc.musl-${process.arch === "arm64" ? "aarch64" : "x86_64"}.so.1` + const glibc = "libc.so.6" + const preferred = (typeof OPENCODE_LIBC === "undefined" ? undefined : OPENCODE_LIBC) === "musl" ? musl : glibc + for (const candidate of [preferred, preferred === musl ? glibc : musl, "libc.so"]) { + try { + return dlopen(candidate, { + inotify_init1: { args: [FFIType.i32], returns: FFIType.i32 }, + close: { args: [FFIType.i32], returns: FFIType.i32 }, + __errno_location: { args: [], returns: FFIType.ptr }, + }).symbols + } catch { + continue + } + } + return +}) + +/** + * Ask the kernel for one inotify instance and immediately give it back. + * + * `@parcel/watcher` builds its shared inotify backend inside the synchronous + * constructor of the N-API `SubscribeRunner`, on whichever thread called + * `subscribe()`. When `inotify_init1` fails there, `InotifyBackend::start()` + * throws before `notifyStarted()`, and `Backend::handleError()` only notifies + * already-registered watchers - so `Backend::run()` keeps waiting on + * `mStartedSignal` and the calling thread never returns. For opencode that + * thread is the JS main thread, which parks the event loop permanently: no + * timer, no fiber and no interrupt can run afterwards, which is why the failure + * produced neither a log line nor an abortable turn. + * + * Returning the errno instead of a boolean keeps the caller honest: EMFILE can + * mean either the per-process descriptor limit or the per-uid + * `fs.inotify.max_user_instances` ceiling, and this cannot tell them apart. + * + * Returns `undefined` when an instance is obtainable, or when the probe itself + * cannot run (no `bun:ffi`, no libc, missing symbols). An unprobeable runtime + * keeps the pre-existing behaviour rather than silently losing file watching. + */ +export function probe(): ProbeFailure | undefined { + const symbols = libc() + if (!symbols) return + + const fd = symbols.inotify_init1(IN_NONBLOCK | IN_CLOEXEC) + if (fd !== -1) { + symbols.close(fd) + return + } + + // errno is thread-local and is clobbered by the next failing libc call, so it + // has to be read before anything else happens on this thread. + const location = symbols.__errno_location() + const errno = location ? read.i32(location) : 0 + const code = errnoCodes.find(([, value]) => value === errno) + return { errno, code: code ? code[0] : "UNKNOWN" } +} diff --git a/packages/core/src/filesystem/watcher.ts b/packages/core/src/filesystem/watcher.ts index c5e20631917e..3953eff45711 100644 --- a/packages/core/src/filesystem/watcher.ts +++ b/packages/core/src/filesystem/watcher.ts @@ -16,6 +16,7 @@ import { Location } from "../location" import { lazy } from "../util/lazy" import { Ignore } from "./ignore" import { Protected } from "./protected" +import type { ProbeFailure } from "./watcher-inotify" declare const OPENCODE_LIBC: string | undefined @@ -54,87 +55,138 @@ export interface Interface {} export class Service extends Context.Service()("@opencode/v2/FileWatcher") {} -const layer = Layer.effect( - Service, - Effect.gen(function* () { - if (yield* Flag.OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER) return Service.of({}) - - const backend = getBackend() - const location = yield* Location.Service - if (!backend) { - yield* Effect.logError("watcher backend not supported", { - directory: location.directory, - platform: process.platform, - }) - return Service.of({}) - } - - const w = watcher() - if (!w) return Service.of({}) - - yield* Effect.logInfo("watcher backend", { directory: location.directory, platform: process.platform, backend }) - const events = yield* EventV2.Service - const fs = yield* FSUtil.Service - const git = yield* Git.Service - const context = yield* Effect.context() - const runFork = Effect.runForkWith(context) - const subscriptions: ParcelWatcher.AsyncSubscription[] = [] - yield* Effect.addFinalizer(() => - Effect.promise(() => Promise.allSettled(subscriptions.map((subscription) => subscription.unsubscribe()))), - ) +export interface LayerOptions { + readonly backend?: ReturnType + readonly watcher?: () => Pick | undefined + readonly probe?: () => ProbeFailure | undefined +} - const callback: ParcelWatcher.SubscribeCallback = (_error, updates) => { - for (const update of updates) { - if (update.type === "create") runFork(events.publish(Event.Updated, { file: update.path, event: "add" })) - if (update.type === "update") runFork(events.publish(Event.Updated, { file: update.path, event: "change" })) - if (update.type === "delete") runFork(events.publish(Event.Updated, { file: update.path, event: "unlink" })) +// Loaded only on the inotify branch: the helper depends on bun:ffi, so a +// runtime without it keeps the pre-existing behaviour rather than losing file +// watching altogether. +const loadProbe = Effect.promise(() => import("./watcher-inotify")).pipe( + Effect.map((module) => module.probe), + Effect.catchCause(() => Effect.succeed(undefined)), +) + +export const layerWith = (options?: LayerOptions) => + Layer.effect( + Service, + Effect.gen(function* () { + if (yield* Flag.OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER) return Service.of({}) + + const backend = options?.backend ?? getBackend() + const location = yield* Location.Service + if (!backend) { + yield* Effect.logError("watcher backend not supported", { + directory: location.directory, + platform: process.platform, + }) + return Service.of({}) } - } - - const subscribe = (directory: string, ignore: string[]) => { - const pending = w.subscribe(directory, callback, { ignore, backend }) - return Effect.promise(() => pending).pipe( - Effect.tap((subscription) => Effect.sync(() => subscriptions.push(subscription))), - Effect.timeout(SUBSCRIBE_TIMEOUT_MS), - Effect.catchCause((cause) => { - pending.then((subscription) => subscription.unsubscribe()).catch(() => {}) - return Effect.logError("failed to subscribe", { directory, cause: Cause.pretty(cause) }) - }), - ) - } - - const config = (yield* (yield* Config.Service).entries()) - .filter((entry): entry is Config.Document => entry.type === "document") - .flatMap((item) => item.info.watcher?.ignore ?? []) - if (location.vcs && (yield* Flag.OPENCODE_EXPERIMENTAL_FILEWATCHER)) { - yield* Effect.forkScoped( - subscribe(location.directory, [...Ignore.PATTERNS, ...config, ...protecteds(location.directory)]), + + const w = (options?.watcher ?? watcher)() + if (!w) return Service.of({}) + + const probe = options?.probe ?? (backend === "inotify" ? yield* loadProbe : undefined) + + yield* Effect.logInfo("watcher backend", { directory: location.directory, platform: process.platform, backend }) + const events = yield* EventV2.Service + const fs = yield* FSUtil.Service + const git = yield* Git.Service + const context = yield* Effect.context() + const runFork = Effect.runForkWith(context) + const subscriptions: ParcelWatcher.AsyncSubscription[] = [] + yield* Effect.addFinalizer(() => + Effect.promise(() => Promise.allSettled(subscriptions.map((subscription) => subscription.unsubscribe()))), ) - } - - if (location.vcs?.type === "git") { - const resolved = (yield* git.repo.discover(location.directory))?.gitDirectory - const vcs = resolved ? yield* fs.realPath(resolved).pipe(Effect.catch(() => Effect.succeed(resolved))) : undefined - if (vcs && !config.includes(".git") && !config.includes(vcs) && (!resolved || !config.includes(resolved))) { - const ignore = (yield* fs.readDirectoryEntries(vcs).pipe(Effect.catch(() => Effect.succeed([])))).flatMap( - (entry) => (entry.name === "HEAD" ? [] : [entry.name]), - ) - yield* Effect.forkScoped(subscribe(vcs, ignore)) + + const callback: ParcelWatcher.SubscribeCallback = (_error, updates) => { + for (const update of updates) { + if (update.type === "create") runFork(events.publish(Event.Updated, { file: update.path, event: "add" })) + if (update.type === "update") runFork(events.publish(Event.Updated, { file: update.path, event: "change" })) + if (update.type === "delete") runFork(events.publish(Event.Updated, { file: update.path, event: "unlink" })) + } } - } - return Service.of({}) - }).pipe( - Effect.catchCause((cause) => { - return Effect.logError("failed to init watcher service", { cause: Cause.pretty(cause) }).pipe( - Effect.as(Service.of({})), - ) - }), - ), -) + const subscribe = (directory: string, ignore: string[]) => + Effect.gen(function* () { + const started = yield* Effect.sync(() => { + const failure = probe?.() + if (failure) return { type: "unavailable" as const, failure } + // Nothing may run between the probe and this call. subscribe() + // reaches the native backend synchronously on the calling thread, + // and a kernel that refuses an inotify instance there parks that + // thread forever instead of failing, so the window in which another + // process can take the last instance stays one expression wide. + return { type: "started" as const, pending: w.subscribe(directory, callback, { ignore, backend }) } + }) + + if (started.type === "unavailable") { + yield* Effect.logWarning("watcher unavailable, continuing without it", { + directory, + backend, + errno: started.failure.errno, + code: started.failure.code, + }) + return false + } + + yield* Effect.forkScoped( + Effect.promise(() => started.pending).pipe( + Effect.tap((subscription) => Effect.sync(() => subscriptions.push(subscription))), + Effect.timeout(SUBSCRIBE_TIMEOUT_MS), + Effect.catchCause((cause) => { + started.pending.then((subscription) => subscription.unsubscribe()).catch(() => {}) + return Effect.logError("failed to subscribe", { directory, cause: Cause.pretty(cause) }) + }), + ), + ) + return true + }) + + const config = (yield* (yield* Config.Service).entries()) + .filter((entry): entry is Config.Document => entry.type === "document") + .flatMap((item) => item.info.watcher?.ignore ?? []) + if (location.vcs && (yield* Flag.OPENCODE_EXPERIMENTAL_FILEWATCHER)) { + const subscribed = yield* subscribe(location.directory, [ + ...Ignore.PATTERNS, + ...config, + ...protecteds(location.directory), + ]) + // The .git watcher below would ask the same exhausted kernel for the + // same resource, so there is nothing left to try. + if (!subscribed) return Service.of({}) + } -export const node = makeLocationNode({ - service: Service, - layer, - deps: [FSUtil.node, Location.node, Config.node, Git.node, EventV2.node], -}) + if (location.vcs?.type === "git") { + const resolved = (yield* git.repo.discover(location.directory))?.gitDirectory + const vcs = resolved + ? yield* fs.realPath(resolved).pipe(Effect.catch(() => Effect.succeed(resolved))) + : undefined + if (vcs && !config.includes(".git") && !config.includes(vcs) && (!resolved || !config.includes(resolved))) { + const ignore = (yield* fs.readDirectoryEntries(vcs).pipe(Effect.catch(() => Effect.succeed([])))).flatMap( + (entry) => (entry.name === "HEAD" ? [] : [entry.name]), + ) + yield* subscribe(vcs, ignore) + } + } + + return Service.of({}) + }).pipe( + Effect.catchCause((cause) => { + return Effect.logError("failed to init watcher service", { cause: Cause.pretty(cause) }).pipe( + Effect.as(Service.of({})), + ) + }), + ), + ) + +export const nodeWith = (options?: LayerOptions) => + makeLocationNode({ + service: Service, + layer: layerWith(options), + deps: [FSUtil.node, Location.node, Config.node, Git.node, EventV2.node], + }) + +export const node = nodeWith() diff --git a/packages/core/test/filesystem/watcher-preflight.test.ts b/packages/core/test/filesystem/watcher-preflight.test.ts new file mode 100644 index 000000000000..43c55e32cdd7 --- /dev/null +++ b/packages/core/test/filesystem/watcher-preflight.test.ts @@ -0,0 +1,124 @@ +import { $ } from "bun" +import { describe, expect } from "bun:test" +import os from "os" +import path from "path" +import { ConfigProvider, Effect, Layer, Logger } from "effect" +import { Config } from "@opencode-ai/core/config" +import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" +import { LayerNode } from "@opencode-ai/core/effect/layer-node" +import { EventV2 } from "@opencode-ai/core/event" +import { FSUtil } from "@opencode-ai/core/fs-util" +import { Watcher } from "@opencode-ai/core/filesystem/watcher" +import { Location } from "@opencode-ai/core/location" +import { AbsolutePath } from "@opencode-ai/core/schema" +import { location } from "../fixture/location" +import { tmpdir } from "../fixture/tmpdir" +import { testEffect } from "../lib/effect" + +const it = testEffect(AppNodeBuilder.build(LayerNode.group([FSUtil.node, EventV2.node]))) + +const configLayer = Layer.succeed( + Config.Service, + Config.Service.of({ + entries: () => Effect.succeed([]), + }), +) + +const flagsLayer = ConfigProvider.layer( + ConfigProvider.fromUnknown({ + OPENCODE_EXPERIMENTAL_FILEWATCHER: "true", + OPENCODE_EXPERIMENTAL_DISABLE_FILEWATCHER: "false", + }), +) + +const WARNING = "watcher unavailable, continuing without it" + +// Builds the watcher against a real git directory with the native binding +// faked, so the only thing under test is what the preflight decides. The +// backend is pinned to inotify because that is the only platform where the +// kernel can refuse the resource. +function build(probe: () => ReturnType>) { + return Effect.gen(function* () { + const tmp = yield* Effect.acquireRelease( + Effect.promise(() => tmpdir()), + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ) + yield* Effect.promise(() => $`git init`.cwd(tmp.path).quiet()) + + const messages: unknown[] = [] + const directories: string[] = [] + + yield* Effect.asVoid(Watcher.Service).pipe( + Effect.provide( + AppNodeBuilder.build( + Watcher.nodeWith({ + backend: "inotify", + probe, + watcher: () => ({ + subscribe: async (directory: string) => { + directories.push(directory) + return { unsubscribe: async () => {} } + }, + }), + }), + [ + [Config.node, configLayer], + [ + Location.node, + Layer.succeed( + Location.Service, + Location.Service.of( + location( + { directory: AbsolutePath.make(tmp.path) }, + { vcs: { type: "git", store: AbsolutePath.make(path.join(tmp.path, ".git")) } }, + ), + ), + ), + ], + ], + ).pipe(Layer.provide(flagsLayer)), + ), + Effect.provide( + Logger.layer([ + Logger.make((options) => { + messages.push(options.message) + }), + ]), + ), + Effect.scoped, + ) + + return { messages, directories } + }) +} + +describe("Watcher preflight", () => { + it.live("degrades to no watcher when the kernel refuses an inotify instance", () => + Effect.gen(function* () { + const result = yield* build(() => ({ errno: os.constants.errno.EMFILE, code: "EMFILE" })) + + // Reaching any assertion at all is the regression: the layer settled + // instead of parking the thread that asked for the instance. + expect(result.directories).toEqual([]) + expect(result.messages.filter((item) => Array.isArray(item) && item[0] === WARNING)).toEqual([ + [ + WARNING, + expect.objectContaining({ + backend: "inotify", + errno: os.constants.errno.EMFILE, + code: "EMFILE", + }), + ], + ]) + }), + ) + + it.live("subscribes as usual when an inotify instance is available", () => + Effect.gen(function* () { + const result = yield* build(() => undefined) + + expect(result.directories.length).toBeGreaterThan(0) + expect(result.messages.filter((item) => Array.isArray(item) && item[0] === WARNING)).toEqual([]) + }), + ) +})