Uh oh!
There was an error while loading. Please reload this page.
fix: an error path with nothing listening on it, five times over - #43
Merged
Conversation
A 42-cell study died at cell 3 on an unhandled pipe error and left no artifact at all. Two cells that had each cost twenty minutes had already succeeded, and both were lost, because the artifact was written once at the end. Three changes, in order of how much they save. The artifact is written after EVERY cell. A file whose run has not finished carries `partial: true` and an `expected` count, so an interrupted study can never be mistaken for a complete one. A cell that throws is one lost cell. `runCell` already returns a blocked row for everything it can foresee; this catches what it cannot, records it as `episode-failed` with the message, and the study continues. `blockedResult` is exported so that row looks like every other refusal instead of being a gap a reader has to notice. `uncaughtException` and `unhandledRejection` write what exists before reporting. The exit code still says the run failed: keeping the data is not the same as pretending the run succeeded. Verified by running a definition whose second cell names a program that does not exist. The first cell is `played`, the second is `blocked: episode-failed` with the spawn error, and the artifact holds both.
Root cause of the same-process replay divergence. `load_state` retries, and every failed attempt issues a `FRAMEADVANCE` that nothing counts. A load that succeeds on the first attempt therefore leaves the emulator one frame earlier than one that succeeds on the second. `reset` then sets `self.frame = self.boot_frame + 1` unconditionally, so the worker's counter is right while the emulator is not. That produces the exact signature the gate reported. Two same-process replays of one boot state and one input log agreed byte for byte to emuFrame 811 and then differed on channel values at IDENTICAL frame numbers, with `ch_c57e_c57f` reading 9000 against 9001. One tick apart is what one uncounted frame does. Identical frame numbering ruled out a miscounted advance, which is what made this hard to see: the count was never wrong, the emulator was. The rate fits too. The gate failed roughly 40% of the time and more often under load, which is when a retry inside `LOAD_STATE` is most likely. The boot instant is now pinned by what it READS AS, not only by its bytes. After the blob is saved, it is restored once and the channels are read back; every later restore must reproduce those values. A restore that lands elsewhere is retried, because the cause is a lost datagram rather than anything about the content, and a worker that cannot re-establish the instant refuses to start a run. Refusing is the point. A run that starts one frame from where it believes it starts produces a log that cannot be replayed, and a divergence discovered later cannot be told from a bad policy. Not reproducible locally: RetroArch is not installed on this machine, so the gate in CI is the test.
The gate refused to run on darwin at all, and its own reason was an architecture claim: "the x86_64 build under Rosetta segfaults during retro_run". Homebrew does install exactly that build, and it does segfault. But `process.platform === 'darwin'` states a broader claim than the evidence supports, and it made the adapter unrunnable off CI for anyone on Apple Silicon. libretro's stable tree ships a universal build with a native arm64 slice. That binary loads an arm64 gambatte core and the free Libbet ROM without incident: `[Gambatte] Plain ROM loaded`, then `Got internal game name: LIBBET`. The guard now asks `lipo -archs` whether the binary has a slice for the host, and refuses only when it does not, naming what it found. A machine that can run the emulator natively is allowed to try. This does not make the gate pass on macOS yet. RetroArch still exits during `reset` there, headless and with both documented `defaults` applied, which is a separate darwin problem from the replay divergence in #40. What changes is that the failure is now reachable and specific instead of hidden behind a skip.
…g package Three adapter gates probe for their Python package with `cwd: tmpdir()`, so the probe cannot import from the repository by accident. A relative `PLAYPROOF_PYTHON` therefore never resolves, the probe fails to execute, and the gate reports the package missing. Measured: `PLAYPROOF_PYTHON=./.venv/bin/python` produced "stable-retro is not importable from ./.venv/bin/python; install it with ... pip install stable-retro" for an interpreter that had stable-retro installed and could list 1006 games. The message advised installing software that was already there, and it was believed. The interpreter is now resolved against the caller's directory before any probe runs. A bare name such as `python3` is left alone so PATH lookup still works. Verified by running the exact command that failed: with the relative path, the stable-retro gate now passes. Also ignores `.pnpm-store/`. Running `pnpm install` inside a container against a mounted repository writes a 116 MB store into the working tree, and it had already reached the index.
`close()` called `removeAllListeners()` on stdout, stderr and stdin, then SIGKILLed the child. That removal took the `error` handler with it. A write already in flight to stdin then completed with EPIPE on a socket that no longer had a listener, and Node turns an unhandled `error` event into a dead process. Measured: three separate long studies died this way, each losing every finished cell, with a stack carrying no frame from this repository. That is why it was hard to attribute: the crash names Node internals and no application code. Only the data listeners are removed now. Every pipe keeps an error listener that swallows what a killed child's pipe is expected to do; the session's outcome is already recorded by `end()`. This is the fourth fault of one shape in this work: an error path with nothing listening on it. The others were a frame-advance barrier that discarded its own failure, a streaming agent that never started and looked identical to one still thinking, and a child's stdout and stderr with no error listener at all.
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Five fixes, four of which are one fault wearing different clothes: an error path with nothing listening on it.
The one that cost the most
close()indrivers/persistent-cli.tscalledremoveAllListeners()on stdout, stderr and stdin, then SIGKILLed the child. That removal took theerrorhandler with it. A write already in flight to stdin completed withEPIPEon a socket that no longer had a listener, and Node turns an unhandlederrorevent into a dead process.Measured: three separate long studies died this way, each losing every finished cell. The stack carries no frame from this repository — only Node internals — which is why it took three occurrences to attribute.
Only the data listeners are removed now; every pipe keeps an error listener for what a killed child's pipe is expected to do.
Finished work now survives unfinished work
The artifact is written after every cell, with
partial: trueand anexpectedcount so an interrupted study is never mistaken for a complete one. A cell that throws becomes oneepisode-failedrow and the study continues.uncaughtExceptionandunhandledRejectionwrite what exists before reporting, and the exit code still says the run failed.Verified by running a definition whose second cell names a program that does not exist: first cell
played, secondblocked, artifact holds both.RetroArch: the replay divergence, root-caused
load_stateretries, and every failed attempt issues aFRAMEADVANCEthat nothing counts, so a load succeeding on the first attempt leaves the emulator one frame earlier than one succeeding on the second.resetthen setsself.frameunconditionally — the worker's counter is right while the emulator is not.That is the reported signature exactly: two replays of one boot state and one input log agreed byte for byte to
emuFrame 811, then differed at identical frame numbers, withch_c57e_c57freading 9000 against 9001. One tick apart is what one uncounted frame does.The boot instant is now pinned by what it reads as, not only by its bytes: restored once at pin time, channels read back, and every later restore must reproduce them. A worker that cannot re-establish the instant refuses to start a run, because a run beginning one frame from where it believes it begins produces a log that cannot be replayed. Tracks #40.
Two guards that stated more than they knew
The macOS skip refused all of darwin while its own comment said "the x86_64 build under Rosetta segfaults" — an architecture claim wearing a platform check. libretro's universal build has a native arm64 slice that loads an arm64 gambatte core and the free Libbet ROM cleanly. The guard now asks
lipo -archsfor a host slice and refuses only when there is none.A relative
PLAYPROOF_PYTHONreported the package missing. Three gates probe withcwd: tmpdir(), so a relative interpreter never resolves and the probe cannot execute.PLAYPROOF_PYTHON=./.venv/bin/pythonproduced "stable-retro is not importable ... install it with pip install stable-retro" for an interpreter that had it installed and could list 1006 games. It is now resolved against the caller's directory; a barepython3still goes through PATH.Also ignores
.pnpm-store/, which a container'spnpm installwrites into a mounted repository (116 MB, already in the index).Local: 20 of 20 test files, boundary check, 0 typecheck errors. ALE and stable-retro gates verified passing natively on macOS.