fix: serialize pretty-mode build progress output under OpenMP#1296
Open
krystophny wants to merge 1 commit into
Open
fix: serialize pretty-mode build progress output under OpenMP#1296krystophny wants to merge 1 commit into
krystophny wants to merge 1 commit into
Conversation
Each target's pretty-mode (TTY) status update is a compound of two console writes, and console_update_line steps the cursor relative to the tracked line count. Previously the two writes sat in separate unnamed critical regions, so a concurrent worker thread could interleave its own output between them and invalidate the relative cursor math, corrupting the rendered progress. Wrap each target's compound update in a named critical(fpm_progress) so it is atomic against other threads. Plain mode (used off a TTY, in CI) is unchanged. Also make the n_complete counter coherent: increment with atomic capture and read with atomic read, instead of an unnamed critical write paired with an unsynchronized read.
This was referenced Jun 4, 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
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
critical(fpm_progress)so it is atomic against other threadsn_completecounter coherent:!$omp atomic captureon incrementand
!$omp atomic readon use, replacing an unnamed critical write pairedwith an unsynchronized read
self-contained write, so per-write atomicity already suffices
Context
Each target's pretty-mode update is a compound of two
console_twrites, andconsole_update_linemoves the terminal cursor relative to the tracked linecount. On
mainthe two writes sit in separate unnamed!$omp criticalregions, so another worker thread can interleave its own output between them,
change
n_line, and invalidate the cursor arithmetic. The result is garbledprogress on an interactive terminal under
-fopenmp.Merge order
Independent PRs (any order):
Stacked chain (merge in order):
Fork-safety fix:
After the above:
Verification
The defect is a non-deterministic data race that only manifests in pretty/TTY
mode under multiple OpenMP threads, so there is no stable automated reproducer.
The before-state is visible in the source on
main: the compound update spanstwo separate unnamed critical regions in
src/fpm_backend_output.f90, andconsole_update_lineinsrc/fpm_backend_console.f90performs cursor movementrelative to a shared, concurrently-mutated
n_line.