Skip to content

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Sep 27, 2025

After different attempts (#15616, #16029, #16168) that exhibited different issues, I finally came up with a dumb abstraction over the ThreadLocal annotation (for quick access) that also injects a reference into Thread.current to keep the value visible to the GC (it can't scan ThreadLocal values).

For targets where the ThreadLocal annotation doesn't work (they could, but that's another issue) we just use the value on Thread.current.

It has the same drawback as #16168 in that once a thread local is declared, the ivar will always be declared on Thread and the compiler won't optimize them away if they're unused (maybe it could, someday). We don't expect dozens of thread locals so a few wasted pointers shouldn't be an issue.

The macro is mostly useful to avoid repeated boilerplate.

There is no support for destructors. If needed, just use a class with a finalizer. When the Thread is collected the thread local value will also be collected, the finalizer will run, hence acting as a destructor.

Related:

  1. drop Thread::Local(T);
  2. introduce the unsafe toggle to the annotation from Feature: Fiber-Local-Storage #15889;

See #16174 and #16175 for use cases (PCRE2, default Random).

This is a simple abstraction over the `ThreadLocal` annotation (for
quick access) that also injects a reference into `Thread.current` to
keep the value visible to the GC (that can't scan `ThreadLocal` values).

There is no support for destructors. When needed just use a class with a
finalizer: when the Thread is collected the thread local value will also
be collected, which will run the finalizer.
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we can still merge this into 1.18.
It adds an undocumented internal helper macro that's never used so far. So it should not have any effect for the release, but we have it in tree so we can move on with the follow-ups (they'll be merged after 1.18 though).

@straight-shoota straight-shoota added this to the 1.18.0 milestone Sep 29, 2025
@ysbaddaden ysbaddaden merged commit 47e0fb2 into crystal-lang:master Oct 3, 2025
39 checks passed
@github-project-automation github-project-automation bot moved this from Review to Done in Multi-threading Oct 3, 2025
@ysbaddaden ysbaddaden deleted the feature/basic-thread_local-macro branch October 3, 2025 09:15
ysbaddaden added a commit that referenced this pull request Oct 3, 2025
This reverts commit 8fda88f.

It's an internal type, we don't plan to use it anymore, and #16173 is
meant to replace it.

It's main feature was the ability to work on every target (unlike the current `@[ThreadLocal]` annotation) and to register destructors... but the annotation can be fixed using the EmulatedTLS LLVM option, and the destructor is crashing on x86_64-darwin when mixing pthreads, unwind and pcre2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants