Repository navigation
Fix timeout races and other issues from the v2 review - #198
Merged
Merged
Conversation
- validate: guard the log stream with a mutex so the timeout thread can't reset it while a test is writing to it, and stop the timeout thread once the tests finish so it can't write its report over the real one. - test: the worker and the watchdog now claim the report with an atomic flag and only the first to claim it writes. The placeholder report is written before the watchdog starts.
- Crash log: back in the per-user temp directory (path computed up front for the signal handler) instead of a shared /tmp file, opened with O_NOFOLLOW. - Crash handler: don't pass null image/symbol names to %s; cast the offset for %ld; include <string.h> and <algorithm>. - --output-filename without --output-dir is resolved against the working directory rather than building a juce::File from a relative path. - pluginval test accepts --name=value options, as validate does. - GitHub Actions: delete each report before running so a stale one from an earlier run is never read. - TestConfig::getPluginPathOrID uses settings_parser::resolvePluginPath. - vst3validator: delete the extracted binary if writing or chmod fails. - Typos, spacing, closing-namespace comments, an em dash, and the JUCE version in CLAUDE.md.
- pluginval test: --record-missing=<value> is an error rather than the value being taken as the config path. - Message-thread helpers share one callOnMessageThreadAndWait() that rethrows an exception on the calling thread, so a throw can't leave the caller waiting forever. Used by the VST3 helpers, the editor helpers, the plugin scan, message-thread tests and the acceptance plugin teardown, which now can't throw out of its destructor. - Child process output is decoded with ChildProcessOutputDecoder, so a UTF-8 character split across two reads is no longer garbled. - operator delete no longer throws (it's noexcept, so that terminated the process); operator new throws std::bad_alloc rather than returning null. Removed the unused violationError(). - Replaced the em/en dashes in CLAUDE.md and the acceptance design doc.
This was referenced Oct 14, 2026
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.
Fixes from a pre-merge review of
v2againstmaster.Races between timeouts and report/log writing
Other fixes
O_NOFOLLOW. The handler no longer passes null names to%s.callOnMessageThreadAndWait(), which rethrows on the calling thread, so a throw can't leave the caller waiting forever. Acceptance plugin teardown can no longer throw out of a destructor.ChildProcessOutputDecoder).operator deleteno longer throws (it'snoexcept, so a throw ended the process).operator newthrowsstd::bad_allocinstead of returning null.pluginval test:--name=valueoptions, asvalidatedoes;--record-missing.--output-filenamewithout--output-diris resolved against the working directory.chmodfails.TestConfigreusessettings_parser::resolvePluginPath.Testing
pluginval run-testspasses, with new tests for--name=valueand the UTF-8 decoder.--timeout-msset to 1 to 200, the report always agreed with the exit code.callOnMessageThreadAndWait()refactor touches every VST3 call made on the message thread, so CI on Linux and Windows matters here.