feat(accessors): add createAccessor factory, move IndirectNThreadHostAccessor into exoproc-accessors - #9
Merged
Merged
Conversation
…Accessor into exoproc-accessors createAccessor(id, options) builds a ready-to-use accessor from a thread id (or process id via options.idType) with a single call, defaulting to IndirectNThreadHostAccessor and optionally wrapping it in NShm for shared memory (options.sharedMemory). createAccessorOptions(aggressiveness) returns a tuned preset (1 = gentle, 2 = balanced + shared memory). Moving IndirectNThreadHostAccessor out of bun-nthread and into exoproc-accessors resolves a real circular workspace dependency: it already needed 9 concrete accessors classes (BootstrapHostAccessor, IndirectCallRedirectorAccessor, MachineCodePoolMiddleware, ...), so createAccessor's own default couldn't live in exoproc-accessors while nthread still depended on it. Fixed nthread.ts's and nshm.ts's HostAccessor imports (both type-only) to use bun-xffi's IHostAccessor interface instead, fully removing their dependency on exoproc-accessors. nhook now imports IndirectNThreadHostAccessor from exoproc-accessors instead of bun-nthread. Also: swapped IndirectNThreadHostAccessor's `instanceof NThread` check for `typeof === 'number'` -- now that NThread and IndirectNThreadHostAccessor live in different packages, a caller's own cross-package-resolved NThread copy could fail instanceof under Wine's known duplicate-module-copy quirk. Reordered the root tsconfig's project references so a clean build resolves in one pass under the new dependency order. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI's format:check step was failing on claude/create-accessor-factory for these 3 files; fixes only reformat, no logic changes.
…sync and pre-initializing createAccessorWithoutInit(id, options) builds the accessor graph exactly as createAccessor used to, without touching init -- the result still lazily initializes on its first real operation, same as before. createAccessor is now async: it calls createAccessorWithoutInit and, when the result is an IInittableAccessor, awaits its init() before resolving. Callers get back an already-initialized accessor instead of paying the lazy-init cost on their first real read/write/call. Updated all tests that hand-built a real IndirectNThreadHostAccessor (`new IndirectNThreadHostAccessor(pid, tid, opts)`) to go through createAccessor/createAccessorWithoutInit instead. Tests that only build mock/local accessors for unit-testing a specific middleware class (e.g. ProcessCacheAccessor, MemsetWriteAccessor mocks) are unchanged -- there's no real pid/thread for the factory to resolve there. nthread.test.ts's two manual RedirectorHostAccessor/NThread/IndirectNThreadHostAccessor(nthread) wirings are also unchanged -- they test that lower-level construction path directly, which createAccessor doesn't cover.
Races an IndirectNThreadHostAccessor per thread in the process, all initializing in parallel -- there's no way to know upfront which thread(s) (if any) will actually return to user mode and land the hijack. Whichever one's init() resolves first wins and is returned; every other candidate is deinitialized in the background once its own init() settles (a no-op for one that never finished or already failed). Only createAccessor supports it -- createAccessorWithoutInit throws, since picking a winner inherently requires attempting init on every candidate and can't be done synchronously. createAccessor races the threads first, then hands the winner to createAccessorWithoutInit as options.backend, so sharedMemory wrapping still applies exactly as it does for the other idTypes.
…adHostAccessor chains processAllThreadIds previously built and initialized a complete IndirectNThreadHostAccessor per candidate thread -- but that chain's onInit() does real extra work beyond landing the hijack (IndirectCallRedirectorAccessor, MachineCodePoolMiddleware, an msvcrt check, opening a remote temp file for FileTransferWriteAccessor, ...), work that only needs to happen once for whichever thread actually wins, not once per candidate. Each candidate now gets just an NThread wired to a RedirectorHostAccessor whose target initially loops back to the NThread itself, so NThread.onInit()'s bootstrap calls land directly on NThread.call() -- the primitive register-context calls the hijack needs, none of the heavier chain. Once a winner's NThread finishes initializing, a real IndirectNThreadHostAccessor is built from it and the redirector is rewired to point at that accessor (the documented pattern for the `backend: NThread` constructor form). createAccessor initializes that one accessor as usual.
…s to 1 createAccessor(id)/createAccessorWithoutInit(id) with no options now treat id as a pid and race every one of its threads by default, instead of treating id as a thread id -- callers no longer need to look up a thread themselves for the common case. createAccessorWithoutInit can't support this idType (it can't pick a winner without attempting init), so it now throws by default unless the caller explicitly passes idType: 'thread'/'process'. createAccessorOptions() now defaults to aggressiveness 1 (gentle: long timeout, no shared memory) instead of 2. Updated tests accordingly: most now pass a pid straight to createAccessor/createAccessorWithoutInit instead of first looking up Thread.getThreads(pid)[0] themselves; a few that assert an exact thread identity now check membership in the process's threads instead, since which thread wins the race is no longer pinned to "the first one enumerated".
createAccessor()/createAccessorWithoutInit() return IHostAccessor, which already exposes every op these tests actually use (call/alloc/read/write/ free/protect/query/scan/machineCode/allocNear) -- the `as IndirectNThreadHostAccessor` casts scattered across the suite existed only to call .deinit(), which isn't on IHostAccessor. Replaced those with an isInittableAccessor(...) runtime guard (the same helper createAccessor itself uses internally) instead of a static cast to the concrete class.
…essThreads The previous implementation let losing candidates run to their own natural init()/timeout instead of stopping them once a winner was found -- CI caught the real consequence: racing multiple threads of the same shared process concurrently, with losers left hammering SuspendThread/SetThreadContext/ ResumeThread on the target for up to their own timeoutMs in the background, destabilized the target badly enough to CallTimeoutError not just the racing test itself but later, unrelated tests reusing the same shared dummy process. NThread already supports this via NThreadOptions.signal (an AbortSignal) -- waitForLanding() honors it during the landing-wait poll loop, and onInit()'s own catch turns an abort into an immediate closeThread() (releases + resumes the thread), the same cleanup path a normal failure takes. Each race candidate now gets its own AbortController wired in as `signal`; as soon as a winner is found, every other candidate is aborted immediately instead of being left to run out its own timeout. A caller-supplied options.nthreadOptions.signal (if any) is composed in too -- either source aborts a candidate.
… return HostAccessor directly
Without options.sharedMemory: true, the result is always a real HostAccessor
(IndirectNThreadHostAccessor or whatever options.backend was), so its init/
deinit/etc. are directly callable -- no isInittableAccessor() runtime check
needed at call sites anymore. Only options.sharedMemory: true still returns
the looser IHostAccessor: the wrapper (NShm by default) is a plain
MiddlewareAccessor with no init/deinit of its own, so that path can't be
narrowed the same way.
Implemented via overloads: the common `options?: { sharedMemory?: false }`
shape resolves to HostAccessor; anything else (sharedMemory: true, or a
non-literal AccessorOptions variable whose sharedMemory can't be statically
narrowed) falls back to IHostAccessor.
Dropped the now-unnecessary isInittableAccessor(...) guards this enabled
across the test suite -- tests call .deinit() directly again, same as before
createAccessor existed, except now backed by the factory.
… always returns one createAccessor()/createAccessorWithoutInit() previously fell back to the looser IHostAccessor type when sharedMemory: true, because the default shared-memory wrapper (NShm) was a plain MiddlewareAccessor with no init/ deinit of its own. Moved HostAccessor itself from exoproc-accessors into bun-xffi (it only ever touched bun-xffi-level members -- InittableMiddleware Accessor, IHostAccessor, isMiddlewareAccessor -- so it didn't actually need to live in the accessors package; exoproc-accessors now just re-exports it for existing consumers). This breaks what would otherwise be a circular dependency (nshm -> accessors -> nshm) and lets NShm extend HostAccessor directly, since bun-nshm already depends on bun-xffi. NShm needed no changes beyond the base class swap -- it never overrode onInit/onInitSync, so it inherits HostAccessor's no-op versions, and gains a real init()/deinit() lifecycle for free: deinit() now cascades to deinit its backend too, and init() (called by createAccessor) now pre-initializes the backend as part of the same chain instead of relying on lazy per-op init. createAccessorWithoutInit/createAccessor now have a single signature each, unconditionally returning HostAccessor -- the sharedMemory-true overload is gone since NShm satisfies it too. AccessorOptions.backend and sharedMemoryProvider are typed as HostAccessor accordingly (a custom backend that isn't already one can be wrapped with `new HostAccessor(...)`). Simplified create-accessor.test.ts's sharedMemory test to call createAccessor directly (no more manually building/tracking a separate base accessor just for cleanup) and dropped the last isInittableAccessor guard.
…child hook NShm goes back to extending MiddlewareAccessor (not HostAccessor) -- that experiment surfaced a real Wine-under-isolated-linker issue where instanceof against a class from another package is unreliable even with a fully current dist/, now documented in CLAUDE.md. HostAccessor itself moves back to bun-xffi (registerChild included) so bun-nthread's NThread can implement the IHostAccessor contract directly without depending back on exoproc-accessors. MiddlewareAccessor's constructor now calls root.registerChild(this) (a no-op on every IHostAccessor except RaceHostAccessor), so a candidate auto-enters a race just by being built with a RaceHostAccessor as its root -- no manual registration call needed. RaceHostAccessor is fully generic (no NThread, AbortController, or timeout knowledge); NThreadRaceHelperAccessor extends it to also cancel a losing candidate's in-flight hijack immediately instead of only cleaning it up after the fact. NThreadRaceAccessor now stands in for the not-yet-known NThread directly inside IndirectNThreadHostAccessor's own chain (its `backend: NThread | NThreadRaceAccessor` constructor form) rather than being built and later wrapped/upgraded after the fact -- so IndirectNThreadHostAccessor is always built exactly once, synchronously, for every idType including processAllThreadIds, and createAccessor/createAccessorWithoutInit no longer need any unwrap logic. `.nthread` becomes a getter that drills through to the resolved NThread once racing settles. sharedMemory: true now splices the shared-memory middleware directly into the resolved accessor's own backend chain and returns that same accessor, instead of wrapping it in a new outer HostAccessor -- preserves the concrete class identity (e.g. `.nthread` stays reachable) that some callers (nhook) need past the generic HostAccessor surface. AccessorOptions.nthreadOptions is renamed to host/hostOptions: `host` picks which HostAccessor class to build (default IndirectNThreadHostAccessor, which keeps its existing processAllThreadIds racing support; a custom host is built directly and doesn't support that idType), `hostOptions` is the now-untyped bag forwarded to it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds a test showing the concrete use case: with createAccessorOptions(2) (sharedMemory: true), a struct's fields can be read/written synchronously (no await) and the writes land in the real cross-process target, verified independently via a raw ReadProcessMemory. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two real bugs surfaced once nthread racing actually exercised concurrent candidates end-to-end: RedirectorHostAccessor's internal placeholder (ThrowingHostAccessor) was constructed with the racing root, so MiddlewareAccessor's constructor auto-registered the placeholder itself as a racer -- it "won" instantly since it never does real work, before any genuine candidate could land. And deinit()/onDeinit() never routed through `target` the way close() already did, so a racing accessor's winning NThread was never actually released, leaving the underlying thread wedged for every later test reusing the same process. Fixed both, then replaced the per-candidate BootstrapHostAccessor proxy objects with a shared root: every candidate NThread now gets the single NThreadRaceAccessor directly as root (auto-registers via registerChild), and a startRacer() hook (added to the generic RacingHostAccessor base) wraps each candidate's init() in an AsyncLocalStorage context so the shared root's call() can resolve back to whichever candidate is currently bootstrapping. bun-nthread is untouched -- NThread's own root-indirection stays generic. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ok demo Replaced the hand-built RemoteCallableMemoryAccessor (reusing notepad.handle with closeHandle: false to dodge a double-CloseHandle) with createAccessor(notepad.pid, createAccessorOptions(2)) -- NThread opens its own thread handle internally, so the handle-reuse workaround is no longer needed, and the demo now exercises the same accessor factory the rest of the project uses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
DummyProcess spawns headless by default (CREATE_NO_WINDOW, no explicit lpDesktop) -- right for tests and most examples, but the notepad-keystroke-hook demo needs an actual window a human can click into and type at. DummyProcessOptions.visible (default false, unchanged behavior) skips CREATE_NO_WINDOW and explicitly points STARTUPINFO.lpDesktop at "winsta0\default" (the interactive window station/desktop), plus STARTF_USESHOWWINDOW/SW_SHOWNORMAL so the window actually shows. Threaded through SpawnStrategy as an optional third parameter -- backward compatible with the existing two-arg strategy in tests/setup.ts. Adds StartupInfoFlags/ShowWindowCommand defines to bun-xffi for this. notepad-keystroke-hook now passes visible: true. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
woldann
force-pushed
the
claude/create-accessor-factory
branch
from
July 18, 2026 07:28
2843a96 to
5d77e5b
Compare
…tgun
NThread.onInit() called getRandomSpinStub()/getRandomPushretStub()/
getRandomJumpStub()/getRandomRetStub()/getRandomAddRsp28RetStub()
directly, without ever awaiting the underlying StubDescriptor's
whenReady(). Each getRandomXStub() swallows StubNotReadyError (and
NoStubFoundError) via its own try/catch and returns undefined either
way, so a caller that reaches onInit() before these process-wide,
eagerly-started background scans (walking kernel32/ntdll/kernelbase's
executable sections) finish gets a misleading NoSleepAddressError/etc.
instead of the real "still scanning" cause. Added
stubs.ts#whenStubsReady(), awaiting every registered descriptor, and
call it first thing in onInit().
Also removed the autoSuspend option: fetchContext()/applyContext()
used it to suspend/resume around every context fetch/apply when not
already suspended, but NThread only ever needs to suspend once, during
onInit()'s landing sequence -- every op after that runs against the
thread while it's parked (running) at the spin stub, relying on EB FE
('jmp $') never touching registers/memory between instructions. The
option only ever added extra suspend/resume cycles beyond that single
setup one, and was never referenced anywhere outside this file.
Corrected pollIntervalMs's doc comment too -- it claimed a default of
50 but the field's actual default was already 2.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
enable()/disable() unconditionally suspended every thread of the target process (Native.Thread.suspendAll) before patching, then resumed them after -- a safety measure against a thread running half-patched bytes. When memory is itself backed by a real NThread hijack (e.g. from createAccessor), that hijacked thread is one of the ones getting suspended here too, via a separate handle. Meanwhile memory.protect()/ write() issue their own SetThreadContext + ResumeThread cycle through that same thread to actually run the call. Both sides act on the one shared OS-level suspend count without knowing about each other's call, so the net count never reaches 0 and the redirected call inside memory.protect() never lands -- CallTimeoutError, reproduced reliably against a single-threaded target (notepad.exe) where the driving thread and the only thread to suspend are the same one. getDrivingThreadId() duck-types memory.nthread.threadId (rather than an instanceof check, unreliable cross-package under Wine's isolated linker) and both methods now exclude it from the set they suspend -- it's already fully under our own control and never executes the target's own code, so it never needed this protection in the first place. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both aggressiveness presets explicitly set pollIntervalMs to 100 (level 1) or 50 (level 2), overriding NThread's own actual default of 2ms with something 25-50x slower -- the doc comment even claimed level 2 already matched "NThread's own defaults" at 50ms, which was wrong. Every redirected call's landing-wait polls at this interval, so the override added real, compounding latency to every single one. Both presets now leave it at NThread's fast default. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… whole demo createAccessor's NThread hijack parks notepad's own thread at our spin stub for as long as the accessor is alive -- on this single-threaded target, notepad's message loop (and so keyboard handling) never runs on its own while that's true. The accessor built at the top of the script was kept alive for the entire session, so notepad was unresponsive to typing the whole time, not just during create/enable. Added withMemory(), building one fresh, short-lived accessor per operation (create/enable/disable, including pause/resume) and deinit()ing it immediately after -- notepad is now only ever unresponsive for the brief moment an operation is actually running. Switched from the hook.enable()/hook.disable() convenience forwarders (which always reuse whatever memory was passed to nhook.create(), since Hook.memory is readonly) to nhook.enable()/nhook.disable() directly, so each call can bring its own fresh accessor. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
createAccessor(id, options) builds a ready-to-use accessor from a thread id
(or process id via options.idType) with a single call, defaulting to
IndirectNThreadHostAccessor and optionally wrapping it in NShm for shared
memory (options.sharedMemory). createAccessorOptions(aggressiveness) returns
a tuned preset (1 = gentle, 2 = balanced + shared memory).
Moving IndirectNThreadHostAccessor out of bun-nthread and into
exoproc-accessors resolves a real circular workspace dependency: it already
needed 9 concrete accessors classes (BootstrapHostAccessor,
IndirectCallRedirectorAccessor, MachineCodePoolMiddleware, ...), so
createAccessor's own default couldn't live in exoproc-accessors while nthread
still depended on it. Fixed nthread.ts's and nshm.ts's HostAccessor imports
(both type-only) to use bun-xffi's IHostAccessor interface instead, fully
removing their dependency on exoproc-accessors. nhook now imports
IndirectNThreadHostAccessor from exoproc-accessors instead of bun-nthread.
Also: swapped IndirectNThreadHostAccessor's
instanceof NThreadcheck fortypeof === 'number'-- now that NThread and IndirectNThreadHostAccessorlive in different packages, a caller's own cross-package-resolved NThread
copy could fail instanceof under Wine's known duplicate-module-copy quirk.
Reordered the root tsconfig's project references so a clean build resolves
in one pass under the new dependency order.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com