[#406] Architecture review remediation: headless automation, plugin lifecycle, CLI dispatch & EDT safety - #407
Merged
Merged
Conversation
- Convert CPU frequency kHz->Hz before resampling; getCPUFrequency() reports kHz, so audio ran ~1000x too slow in real emulation - Read selected register from 0xFFFD (address/latch), not write-only data port 0xBFFD, per ZX Spectrum wiring - Envelope step is 32*EP CPU cycles (fE=clock/256/EP over 16 steps), not 512*EP which ran 16x too slow Tests updated to pass kHz; add sample-rate regression test. Refs #398 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix correctness bugs in the headless/automation path and tighten module boundaries, plugin lifecycle, build and docs. - Headless automation NPE: ApplicationApiImpl allows null GUI - CLI dispatch: Runner -> Callable<Integer>, drop double-parse flag - Automation: guard wait() against lost-notification hang - VirtualComputer: reverse-order plugin destroy() + close classloader - New EmulationProgress port removes runtime -> GUI import leak - EDT-safe CPU status panels (8080, brainduck, ram, rasp, ssem, z80) - byte-mem: volatile memory/bank fields for cross-thread visibility - Pin Java 11 toolchain; add checkNoSnapshots/releaseDist tasks - Add docs/adr with ADR-0001..0007 - Tests: AutomationBoundaryTest plus updated runtime/CLI suites Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
vbmacher
changed the title
RunnerTest exercises `Runner.executeArgs("automation", "--help")`.
AutomationCommand had no --help option, so the arg was unmatched and,
with `collectErrors(true)` set on the executed CommandLine, picocli's
RunLast fell through to running the parent `Runner.call()`, which builds
the Swing GUI and shows the modal computer-chooser dialog. Under Cacio
(headless=false) the modal dialog blocked forever, hanging the test JVM
and the whole `./gradlew build` (2h+ on CI vs ~2.5min normally).
- AutomationCommand: add mixinStandardHelpOptions so `automation -h/--help`
prints subcommand help and returns 0 without touching the GUI.
- Runner.createCommandLine: drop collectErrors(true). It is only useful
for lenient parse inspection but makes execute() silently launch the
GUI on any subcommand parse error; normal handling now prints usage
and returns a non-zero exit code instead.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
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.
Overview
Primarily remediates findings from a full architecture & implementation review of the runtime and plugin platform (tracked in #406): fixes correctness bugs in the headless/automation path, tightens module boundaries and plugin lifecycle, pins the build toolchain, and bootstraps
docs/adr.Two smaller, pre-existing commits ride along on this branch — see Also included below.
[#406] Architecture review remediation
Bug fixes (correctness)
ApplicationApiImplno longer rejects a null GUI (this.gui = gui), matching the plugin contract where CPU plugins already guard ongetGUI() == null. FixesNullPointerExceptioninautomation --no-gui.Runnernow implementsCallable<Integer>with a single picocli execution (executeArgs/normalizeArgs), removing the double-parse +runsSomeCommandflag that made inherited options placed after theautomationsubcommand silently launch the GUI. Also removes scatteredSystem.exit.AutomationguardsresultStateLock.wait()with a state predicate so a fast-halting program cannot notify before the wait and hang forever.Boundaries & lifecycle
VirtualComputer.close()destroys plugins in reverse init order (devices → CPU → memory → compiler), idempotently, then owns and closes the pluginURLClassLoader, then closes config.EmulationProgressport (withNONEno-op);Automationno longer importsgui.dialogs.AutoDialog, which now implements the port.8080,brainduck,ram,rasp,ssem,z80).volatile.Build & docs
JavaLanguageVersion.of(11).checkNoSnapshots+releaseDistGradle tasks fail release packaging on any-SNAPSHOTdependency.docs/adr/with ADR-0001…0007 (ADR process, plugin-loading contract, context interface hashing, project format, headless mode, plugin lifecycle & classloader ownership, automation-progress UI boundary).docs/code.adoc/docs/components.adocaligned re: context registration.Tests
New
AutomationBoundaryTest; updatedAutomationTest,RunnerTest,AutomationCommandTest,ApplicationApiImplTest,VirtualComputerTest(destroy order + classloader close),PluginLoaderTest.Also included (separate commits on this branch)
Ay38910Chip.java+ test).Testing
./gradlew buildemuStudio -cn BrainDuck automation --no-gui -w 800completes without NPE.Notes
EMUSTUDIO_IDreconciliation; positional plugin/config pairing;trustedPluginrename; emuLib-side items (CPUWatchTaskinterrupt notify,getGUI()nullability javadoc).feature-314, but issue Implement ZX Spectrum 48K #314 is unrelated ("Implement ZX Spectrum 48K"); the remediation work is tracked under Architecture review remediation: headless automation, plugin lifecycle, CLI dispatch & EDT safety #406.Closes #406