Skip to content

Safe lazy initialization of class properties#16756

Draft
ysbaddaden wants to merge 4 commits intocrystal-lang:masterfrom
ysbaddaden:feature/safe-lazy-initialization-of-class-properties
Draft

Safe lazy initialization of class properties#16756
ysbaddaden wants to merge 4 commits intocrystal-lang:masterfrom
ysbaddaden:feature/safe-lazy-initialization-of-class-properties

Conversation

@ysbaddaden
Copy link
Copy Markdown
Collaborator

@ysbaddaden ysbaddaden commented Mar 19, 2026

Wraps the lazy initialization of a class variables in a rwlock so the variable will only be (re)initialized once when it's nil.

  • Prevents multiple initializations if the block yields (concurrent bug).
  • Prevents multiple initializations in MT environment (parallel bug).
  • Prevents initialization reentrancy (raises on deadlock).
  • No known regression (see Regression in untyped getter macro #15556).

Uses a simple lock instead of trying to reuse crystal/once that led to the variable not being reinitialized when reset back to nil.

TODO:

ISSUE: it's still parallel unsafe because the class_setter will mutate the class variable without synchronization and the class variable might be a mixed union and thus unsafe to store/load (see #15085)... but the setter doesn't know if there's (or will be) a lazy initialization. I could fix class_property(&) to have the setter synchronize... but it wouldn't be equivalent to class_getter(&) + class_setter that would still be unsafe.

I believe we must start fixing #15085 for class variables so we can at least trust that (value = @@foo).nil? and @@foo = value are thread safe.

Closes #14905
Alternative to #15548 (reverted)

@ysbaddaden ysbaddaden mentioned this pull request Mar 20, 2026
@ysbaddaden ysbaddaden marked this pull request as ready for review March 20, 2026 18:34
straight-shoota pushed a commit that referenced this pull request Mar 24, 2026
Always checked alternative to `Sync::Mutex` and `Sync::RWLock` in a single and significantly smaller type (no crystal type id, no lock type, no reentrancy counter) at 24 bytes instead of 40 bytes. The API is also limited to `#lock(&)` and `#rlock(&)` and it raises on deadlock.

Extracted from #16756.

NOTE: this is for specific internal usages where we want simple safety; we won't need an unchecked lock (use `Sync::MU` directly) or a reentrant lock (use `Sync::Mutex` or `Sync::RWLock`) types.
@ysbaddaden
Copy link
Copy Markdown
Collaborator Author

Back to DRAFT. We need to think deeper about the issue.

@ysbaddaden ysbaddaden marked this pull request as draft March 29, 2026 15:27
@ysbaddaden ysbaddaden moved this from Review to In Progress in Multi-threading Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Ensuring class_getter runs exactly once under concurrent access

1 participant