gh-769: Replace celix_tss usage with libuv key#818
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #818 +/- ##
==========================================
+ Coverage 91.36% 91.40% +0.03%
==========================================
Files 235 235
Lines 28686 28656 -30
==========================================
- Hits 26209 26193 -16
+ Misses 2477 2463 -14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I am on vocation and will be back on the 19th. Then I will investigate on this. |
|
I just studied the Win32 API of TLS, and found that it maps nicely to our current implementation:
For Win32 support, implementing WDYT? @pnoltes Note that option 4 does not work because we cannot rely on a "shadow" |
I agree, so this means we will reject this PR. I do think it is wise to move the tss api to a separate header (instead of the threads header). |
The issue we encountered just shows that our threading API does not extend well to Windows, so replacing it is OK. Another option is C++11 |
…v-key' into feature/769-replace-tss-with-uv-key
This PR replaces the custom
celix_tssimplementation withlibuv'suv_key_tAPI.Current Status
This is a draft because the
libuvTLS implementation lacks a destructor callback, which was previously used to clean upcelix_err_tstructures. Specifically,pthread_key_createallows a destructor registration, butuv_key_createdoes not (likely due to limitations in the underlying Windows TLS implementation).The Problem
Without a destructor,
celix_err_tstructures allocated viamallocincelix_err_getTssErr()are leaked when a thread terminates.Proposed Paths Forward
I have identified four potential strategies to address the leak:
Global Registry: Store created
celix_errstructs in a mutex-protected list. This list is cleaned up in a__attribute__((destructor))function.Thread Monitoring: Store thread IDs in the
celix_errstructure and provide a periodic cleanup function that usespthread_kill(tid, 0)to check if threads are still alive.Consume-to-Cleanup + Global Registry: Free the TLS entry whenever the last error message is consumed (popped). Additionally, include Option 1 to ensure any unconsumed errors are freed at program exit.
malloc/freecalls during error-heavy operations and theoretically does not solve the growing memory issues (if users are not consuming celix_err messages).Platform-Specific Hybrid + (future) Global Registry: Use
libuvkeys for data access but register a "shadow"pthread_keyon Linux/macOS solely to trigger a destructor on thread exit (using a#ifndef _WIN32). For Windows, this would fall back to Option 1 behavior.Current Preference: Option 3.