Skip to content

improve concurrency safety for Compiler.finish! #57248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 8, 2025

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Feb 3, 2025

Similar to #57229, this commit ensures that Compiler.finish! properly synchronizes the operations to set max_world for cached CodeInstances by holding the world counter lock. Previously, Compiler.finish! relied on a narrow timing window to avoid race conditions, which was not a robust approach in a concurrent execution environment.

This change ensures that Compiler.finish! holds the appropriate lock (via jl_promote_ci_to_current).

@aviatesk aviatesk requested a review from vtjnash February 3, 2025 20:13
@oscardssmith oscardssmith added multithreading Base.Threads and related functionality bugfix This change fixes an existing bug compiler:inference Type inference labels Feb 3, 2025
@aviatesk aviatesk force-pushed the avi/staticdatarace-finish! branch from 3196233 to aa3337c Compare February 4, 2025 04:01
@Keno Keno added the backport 1.12 Change should be backported to release-1.12 label Feb 5, 2025
@Keno Keno force-pushed the kf/staticdatarace branch from fc65ba4 to 16f48a0 Compare February 6, 2025 02:40
@KristofferC KristofferC mentioned this pull request Feb 6, 2025
32 tasks
Base automatically changed from kf/staticdatarace to master February 6, 2025 19:31
Similar to #57229, this commit ensures that
`Compiler.finish!` properly synchronizes the operations to set
`max_world` for cached `CodeInstance`s by holding the world counter
lock. Previously, `Compiler.finish!` relied on a narrow timing window to
avoid race conditions, which was not a robust approach in a concurrent
execution environment.

This change ensures that `Compiler.finish!` holds the appropriate lock
(via `jl_promote_ci_to_current`).
@aviatesk aviatesk force-pushed the avi/staticdatarace-finish! branch from aa3337c to f503808 Compare February 7, 2025 06:35
@aviatesk aviatesk merged commit 4ebb50b into master Feb 8, 2025
4 of 7 checks passed
@aviatesk aviatesk deleted the avi/staticdatarace-finish! branch February 8, 2025 04:14
KristofferC pushed a commit that referenced this pull request Feb 11, 2025
Similar to #57229, this commit ensures that
`Compiler.finish!` properly synchronizes the operations to set
`max_world` for cached `CodeInstance`s by holding the world counter
lock. Previously, `Compiler.finish!` relied on a narrow timing window to
avoid race conditions, which was not a robust approach in a concurrent
execution environment.

This change ensures that `Compiler.finish!` holds the appropriate lock
(via `jl_promote_ci_to_current`).

(cherry picked from commit 4ebb50b)
KristofferC added a commit that referenced this pull request Feb 13, 2025
Backported PRs:
- [x] #57142 <!-- Add reference to time_ns in time -->
- [x] #57241 <!-- Handle `waitpid` race condition when `SIGCHLD` is set
to `SIG_IGN` -->
- [x] #57249 <!-- restore non-freebsd-unix fix for profiling -->
- [x] #57211 <!-- Ensure read/readavailable for BufferStream are
threadsafe -->
- [x] #57262 <!-- edit NEWS for v1.12 -->
- [x] #57226 <!-- cfunction: reimplement, as originally planned, for
reliable performance -->
- [x] #57253 <!-- bpart: Fully switch to partitioned semantics -->
- [x] #57273 <!-- fix "Right arrow autocompletes at line end"
implementation -->
- [x] #57280 <!-- dep: Update JuliaSyntax -->
- [x] #57229 <!-- staticdata: Close data race after backedge insertion
-->
- [x] #57298 <!-- Updating binding version to fix MMTk CI -->
- [x] #57248 <!-- improve concurrency safety for `Compiler.finish!` -->
- [x] #57312 <!-- Profile.print: de-focus sleeping frames as gray -->
- [x] #57289 <!-- Make `OncePerX` subtype `Function` -->
- [x] #57310 <!-- Make ptls allocations at least 128 byte aligned -->
- [x] #57311 <!-- Add a warning for auto-import of types -->
- [x] #57338 <!-- fix typo in Float32 random number generation -->
- [x] #57293 <!-- Fix getfield_tfunc when order or boundscheck is Vararg
-->
- [x] #57349 <!-- docs: fix-up world-age handling for META access -->
- [x] #57344 <!-- Add missing type asserts when taking the queue out of
the task struct -->
- [x] #57348 <!-- 🤖 [master] Bump the SparseArrays stdlib from 212981b
to 72c7cac -->
- [x] #55040 <!-- Allow macrocall as function sig -->
- [x] #57299 <!-- Add missing latestworld after parameterized type alias
-->
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Feb 13, 2025
serenity4 pushed a commit to serenity4/julia that referenced this pull request May 1, 2025
Similar to JuliaLang#57229, this commit ensures that
`Compiler.finish!` properly synchronizes the operations to set
`max_world` for cached `CodeInstance`s by holding the world counter
lock. Previously, `Compiler.finish!` relied on a narrow timing window to
avoid race conditions, which was not a robust approach in a concurrent
execution environment.

This change ensures that `Compiler.finish!` holds the appropriate lock
(via `jl_promote_ci_to_current`).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug compiler:inference Type inference multithreading Base.Threads and related functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants