Skip to content

Conversation

Copy link

Copilot AI commented Jan 4, 2026

The lock-free ThreadLocalStorage implementation using InterlockedCompareExchangePointer was identified as not entirely thread-safe. Replaced with lock-per-bucket approach using SRW locks.

Changes

  • Bucket structure: Each hash bucket now contains a wil::srwlock and unique_process_heap_ptr<Node> head pointer
  • Read path: Acquires shared lock for concurrent traversal
  • Write path: Acquires exclusive lock with double-check to prevent race between read-lock release and write-lock acquisition
  • Memory management: Replaced manual HeapAlloc/HeapFree with unique_process_heap_ptr for RAII cleanup
  • Test coverage: Added multi-threaded test (10 threads × 1000 iterations) validating concurrent access

Implementation

// Before: Lock-free with interlocked operations
for (auto pNode = m_hashArray[index]; pNode != nullptr; pNode = pNode->pNext) {
    if (pNode->threadId == threadId) return &pNode->value;
}
// Insert with InterlockedCompareExchangePointer...

// After: Lock-per-bucket with double-check
{
    auto lock = bucket.lock.lock_shared();
    // Search with shared lock...
}
// Allocate outside lock
auto lock = bucket.lock.lock_exclusive();
// Double-check for race, then insert

The shared lock overhead on the read path (~5-10ns) is negligible compared to thread safety benefits. Exclusive lock is acquired once per thread per bucket.

Original prompt

This section details on the original issue you should resolve

<issue_title>Revisit lock-free ThreadLocalStorage storage management</issue_title>
<issue_description>https://github.com/microsoft/wil/blob/master/include/wil/result.h#L403-L430

Internal analysis suggests that this sequence is not entirely thread safe. Some options include:

  • Use an srwlock-per-bucket (costs a function call)
  • Figure out the right sequence of acquire/release semantics for reading the hashtable bucket entries' starting pointer
  • Spin gently (using the platform's "yield" instructions as necessary)
  • Deeper analysis to prove safety with annotations and documentation

Thanks to @sudhakar-vp-zz and @cpkleynhans for the analysis! (This is http://task.ms/58950594 for future explorers.)</issue_description>

<agent_instructions>Read the code linked in the first comment, and replace its implementation with the code in the gist of the second comment. Use the existing type names (renaming the gist code as necessary). If there isn't already a test case validating this type, add one that uses multiple threads trying to access the data structure at once.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

@jonwis See potential approach for "lock per bucket" here - https://gist.github.com/jonwis/f50d8f3d28a85a074cb74aa13d4c774c ... the additional AcquireSrwLockShared call to read the bucket field is "cheapish" and the "Exclusive" happens once per thread on its bucket.

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits January 4, 2026 00:38
…d multi-threaded test

Co-authored-by: jonwis <18537118+jonwis@users.noreply.github.com>
…g exclusive lock

Co-authored-by: jonwis <18537118+jonwis@users.noreply.github.com>
Co-authored-by: jonwis <18537118+jonwis@users.noreply.github.com>
Copilot AI changed the title [WIP] Revisit lock-free ThreadLocalStorage storage management Replace lock-free ThreadLocalStorage with SRW lock-per-bucket implementation Jan 4, 2026
Copilot AI requested a review from jonwis January 4, 2026 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Revisit lock-free ThreadLocalStorage storage management

2 participants