internal: no_std thread local - #6356
Conversation
53c7ed4 to
453afd9
Compare
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks, generally seems fine, a couple of thoughts.
I have a feeling that due to the way we currently implement Python::attach, even if we migrate to PEP 788 APIs (which we should definitely do) then we'll still need thread-locals to handle internal state. Maybe if we exposed a more complex (deliberate) thread-state API to users, we could avoid the thread locals, but that isn't a clear enough idea in my head nor am I sure it'd be worth the churn for most users.
| cfg_select! { | ||
| Py_LIMITED_API => { | ||
| // SAFETY: inner is returned by PyThread_tss_alloc and is not used after this call | ||
| unsafe { PyThread_tss_free(self.inner.load(Ordering::SeqCst)) }; | ||
| }, | ||
| _ => { | ||
| // SAFETY: inner is not used again after this call | ||
| unsafe { PyThread_tss_delete(self.inner.get()) }; | ||
| }, | ||
| } |
There was a problem hiding this comment.
I guess in both of these cases all the values stored for each thread are leaked?
Is there a way that we can trigger destruction of thread-local values on native thread exit? It might not matter for the types which we currently store in thread locals, maybe not ever. So this might just be a question for sake of curiosity which we could document here.
There was a problem hiding this comment.
It would require calling platform-specific APIs.
|
While I was working on the requested changes, I found another possible race condition. Thread A checks the state and sees that it's accessible. I have no idea if this is actually possible, but the only fix I can think of is not to have a destructor and leak the tss key. |
26cdcb4 to
4a5424c
Compare
3 participants
I've re-implemented
LocalKeyin terms of python's tss api.There is currently only one place that uses thread locals, and that is
internal/state.rs. It stores theATTACH_COUNTin a thread localCell. If the plan is to eventually re-write that file using thePyThreadStatefunctions, it can probably be done without using thread local storage at all.