Serialize Qt log file writes to fix macOS tune crash#2284
Merged
ten9876 merged 1 commit intoten9876:mainfrom May 2, 2026
Merged
Serialize Qt log file writes to fix macOS tune crash#2284ten9876 merged 1 commit intoten9876:mainfrom
ten9876 merged 1 commit intoten9876:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Thanks @jensenpat — this is a tightly scoped, well-justified fix and the root-cause writeup matches what's in the diff.
A few things I checked and liked:
- Leaked-mutex pattern matches existing convention. The
static QMutex* mutex = new QMutexmirrors the leakedQRegularExpression*pattern already inredactPii()(around lines 74–90), with the same shutdown-ordering rationale called out in the comment. That keeps the file internally consistent. - Lock covers redaction + file write + stderr. Holding the lock across
redactPii()is slightly stronger than strictly required (theQRegularExpressionobjects are documented as thread-safe formatch), but it's the right call here because it also serializes thes_logFileandstderrwrites from a single critical section, which is what the crash report shows is needed. - Drops
QTextStream(s_logFile)per call. Re-wrapping the sameQFile*in a freshQTextStreamon every message is exactly the shape that corrupts Qt's internal write buffer state, so going toQFile::write(lineBytes)+flush()is the right simplification, not just a serialization change. lineBytesreused for stderr. Switching the stderr path fromtoLocal8Bit()totoUtf8()is a minor behavior change, but AetherSDR's supported platforms (Linux/macOS) are UTF-8 terminals, so this is fine and saves a re-encode.
No null/RAII concerns, the change is contained to src/main.cpp as the PR title implies, and the radio command path is correctly left untouched.
Looks good to me from a review standpoint — leaving final merge to the maintainer.
ten9876
added a commit
that referenced
this pull request
May 2, 2026
The v0.9.5.1 entry was originally cut for just the TCI TX hotfix (#2285), but six additional fixes had already landed in main between the v0.9.5 tag and v0.9.5.1: - #2275 NR2 wisdom generation off the audio thread - #2278 SmartLink disconnect teardown - #2279 Reset RX slice tabs on disconnect (#2254) - #2280 macOS panadapter pop-out refresh + multi-pan dock layout - #2282 SmartLink reconnect after WAN drop - #2284 Qt log handler serialization fixes macOS tune-time crash CHANGELOG.md gets a per-fix entry with root cause + behavior change. WhatsNewData.cpp regenerated via scripts/gen_whatsnew.py so the in-app What's New dialog reflects the full v0.9.5.1 contents. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
ten9876
added a commit
that referenced
this pull request
May 2, 2026
Match the v0.9.5 `(#NNNN, contributor)` heading style and call out @rfoust and @jensenpat in the summary for the bulk of this release. Authors per PR: - @rfoust: #2278, #2279, #2280, #2282 (SmartLink + macOS popout) - @jensenpat: #2275, #2284 (NR2 wisdom + Qt log serialization) WhatsNewData.cpp regenerated. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
ten9876
added a commit
that referenced
this pull request
May 3, 2026
When AetherSDR is launched from OpenDeck (or any other parent that captures stderr but doesn't drain it — Stream Deck "Run Command", systemd user services, GUI launchers), the ~64 KB pipe buffer fills after about 60 KB of debug output and the next fprintf(stderr, ...) blocks indefinitely. Worse, since #2284 the entire messageHandler ran under a global QMutex to serialize log-file writes, so once one thread blocked on stderr every other thread that called qDebug / qInfo / qWarning queued behind it on the mutex — full-app deadlock, no log output, threads sitting in futex_wait. Two changes: 1. Move the file write into a tighter scope so the mutex is only held across the QFile call. The fprintf is outside the mutex — even an unexpected stderr stall can no longer block other threads' log calls. 2. Skip the stderr mirror entirely when stderr is not a TTY. CLI launches keep their console output; Stream Deck / systemd-user / GUI launchers go straight to the log file. The log-file path is unchanged in both cases. isatty / fileno are POSIX; on Windows they live in <io.h> as _isatty / _fileno. Wrap accordingly. Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
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.
Summary
QTextStreamwrapping with a direct UTF-8QFile::write()/flush()path.Root Cause
The macOS crash report aborts in libmalloc while Qt is appending a log line:
QDebug::~QDebug()->MainWindow::logTunePolicyDecision()-> Qt message handler ->QTextStreamPrivate::flushWriteBuffer()->QFileDevice::writeData()->QRingBuffer::append()-> malloc abort.qInstallMessageHandlercan receive messages from any Qt thread, but AetherSDR's handler wrote every log message through one globalQFile*without synchronization. ConcurrentqDebug()/qCInfo()output from the main thread and worker threads can corrupt Qt's internal file/write buffer state. The tune policy diagnostic line in the crash report is the log message that exposed the corruption; the failing object is the logging sink, not the Flex tune command path.The fix adds one intentionally leaked mutex, mirroring the existing shutdown-safe treatment of the redaction regexes, and holds it across PII redaction, file write, and stderr write so the handler is fully serialized.
Radio Behavior
This PR does not change radio command ordering or panadapter policy. The existing
applyTuneRequest()flow still sendsslice tune ... autopan=0, optionally applies local pan status, and then sends the samedisplay pan set ... center=...command when the reveal/follow policy decides it is needed. I checked the local FlexLib v4.2.18 references forslice tuneanddisplay pan setwhile confirming this stayed outside the radio API path.Validation
cmake --build build --parallel 10ctest --test-dir build --output-on-failure -j10git diff --checkLocal app bundle from validation build:
/private/tmp/AetherSDR-macos-log-crash-root-cause/build/AetherSDR.app👨🏼💻 Generated with OpenAI Codex (GPT-5.5 Pro 4/23) and tested by @jensenpat