Skip to content

Add thread safety to at_exit#15598

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
ysbaddaden:fix/add-thread-safety-to-at-exit
Mar 27, 2025
Merged

Add thread safety to at_exit#15598
straight-shoota merged 1 commit intocrystal-lang:masterfrom
ysbaddaden:fix/add-thread-safety-to-at-exit

Conversation

@ysbaddaden
Copy link
Collaborator

@ysbaddaden ysbaddaden commented Mar 25, 2025

Protects the @@handlers class variable of Crystal::AtExitHandlers with a thread mutex.

Avoids a parallel initialization that could create two distinct arrays, and lose one at_exit handler;

Avoids a parallel mutation of the array when at_exit is called in parallel to running the handlers, or when at_exit is called by an at_exit handler 🤷 In these edge cases, the handler is merely added and will be run next.

There should be no behavior change from not using the mutex compared to ST.

I didn't branch based on the preview_mt flag because the Mutex should usually not be contended and its impact be negligible overall. That allowed to keep the implementation simple.

@ysbaddaden ysbaddaden added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:runtime topic:multithreading labels Mar 25, 2025
@ysbaddaden ysbaddaden self-assigned this Mar 25, 2025
@straight-shoota
Copy link
Member

when at_exit is called by an at_exit handler

How would that cause any concurrent mutation?

@straight-shoota straight-shoota added this to the 1.16.0 milestone Mar 25, 2025
@straight-shoota straight-shoota merged commit 59c382f into crystal-lang:master Mar 27, 2025
35 checks passed
@github-project-automation github-project-automation bot moved this from Review to Done in Multi-threading Mar 27, 2025
@ysbaddaden ysbaddaden deleted the fix/add-thread-safety-to-at-exit branch March 27, 2025 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:multithreading topic:stdlib:runtime

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants