fix(extensions): recover worker after failed texture setup - #259
Open
Souptik96 wants to merge 1 commit into
Open
fix(extensions): recover worker after failed texture setup#259Souptik96 wants to merge 1 commit into
Souptik96 wants to merge 1 commit into
Conversation
A generation with enable_texture builds the texture pipeline lazily, and extensions free the shape pipeline first to make room for it. When that setup failed (missing xatlas being the common case) the generator was left with _model = None while the worker process stayed alive, and nothing reset the loaded state: - runner.py called gen.generate() unconditionally, with no loaded check. - ExtensionProcess._loaded stayed True, because it is only cleared by unload(), stop() and the cancel hard-kill path, not by a failed run. - GeneratorRegistry.get_active() therefore skipped load(). Every later generation then raised "TypeError: 'NoneType' object is not callable" until the worker was killed by hand. The runner now ensures the model is loaded before inference, mirroring what get_active() already does host-side, and reports its post-failure loaded state so ExtensionProcess can drop its cached flag and reload on the next run. The original failure is still surfaced unchanged. Fixes lightningpixel#239
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.
Fixes #239
Root cause
A generation with
enable_texturebuilds the texture pipeline lazily, and extensions free the shape pipeline first to make room for it. When that setup fails — a missingxatlasbeing the case reported in #239 — the generator is left with_model = Nonewhile the worker subprocess stays alive.Nothing then resets the loaded state, at either layer:
api/runner.pycalledgen.generate()unconditionally in itsgeneratebranch, with no check that a model was actually in memory.ExtensionProcess._loadedstayedTrue. It is only cleared byunload(),stop()and the cancel hard-kill path — never by a failed generation.GeneratorRegistry.get_active()only callsload()if not gen.is_loaded(), so it skipped the reload.Both layers therefore believed the worker was loaded while
_modelwasNone, and every later generation died onself._model(...)with:…until the worker was killed by hand, exactly as the issue describes.
Fix
Minimal, and follows the pattern already used elsewhere in the codebase (the cancel hard-kill path drops state precisely so
the model will reload on next run):api/runner.py—_ensure_model_loaded(gen)runs before inference and reloads when the model is gone, mirroring whatGeneratorRegistry.get_active()already does host-side. A reload emits a warning log so the cause stays visible rather than being silently papered over.api/runner.py— theerrorpayload now carriesloaded, the worker's post-failure state, read through_generator_is_loaded()which never raises (a half-initialised generator can makeis_loaded()itself throw, and an unreadable state is reported as not loaded so the caller reloads).api/services/extension_process.py— on an error reportingloaded: false,_loadedis cleared soget_active()reloads before the next run instead of reusing a broken worker.The original failure is still surfaced unchanged: the first attempt fails with the real cause (the missing
xatlas), and it is the retry that now succeeds. No automatic in-run retry or backoff was added, since the failure is usually a missing dependency that will not fix itself within a run — recovery is offered on the next explicit attempt instead.Verification
Test command is the repo's own:
python -m unittest discover -s testsfromapi/(whatnpm run test:pyinvokes).api/tests/test_runner.pygains a driver that runsrunner.main()end-to-end against a throwaway extension whose lazy texture setup fails on the first attempt and succeeds on the second — the exact sequence from the issue — asserting the first run errors with the real cause, the retry returnsdone, and_modelis genuinely set again afterwards.Negative control — with the tests in place but both source changes reverted, 8 of the new tests fail, and the retry reproduces the reported symptom verbatim:
With the fix applied:
Ran 59 tests — OK (skipped=2). Four of the twelve new tests are deliberate invariant controls (a failure that keeps its model must keep_loaded; an error must still propagate its traceback; a successful run must not reload) and pass in both states.Not verified
The reporter's environment (RTX 5090 / CUDA 13.3 / real
xatlastexgen) was not available to me, so the failure is reproduced through a generator that models the documented_model = Nonelifecycle rather than a real texture pipeline. The changed code paths are hardware-independent — protocol and loaded-state bookkeeping only. The JS/TS suites and ESLint were not run as no JS/TS files are touched.