fix(threading): give every thread a distinct ManagedThreadId (#1958, SR-AUD-193)

Thread::CurrentThread().getManagedThreadIdProperty() returned the literal 1
from every thread not created through a System::Threading::Thread object --
the main thread, every raw std::thread, every std::async worker, every pool
thread -- so all of them collided with each other and with the main thread.
.NET's ManagedThreadId is "a unique identifier for the current managed
thread" with no path returning a shared constant, so anything keyed on it
was keyed on the single value 1.

The id is now a thread_local assigned on first use, drawn from
nextManagedId_ -- the SAME counter Thread's constructor uses. No signature,
layout, vtable or noexcept change; no consumer rebuild. Landed under SA-5.

The main thread keeps 1 by IDENTITY rather than by arriving first: its OS id
is captured during static initialisation, which runs on it. "Give 1 to
whoever asks first" hands it to a worker whenever one asks before main does,
which fails intermittently rather than consistently.

One counter, not two. Uniqueness is across all threads, not within each
kind; a separate external counter gives each kind internally distinct ids
and still lets the two kinds collide. That was mutation M4, and it survived
the first three tests. The obvious test for it does not work either:
collecting four ids of each kind and asserting eight distinct values PASSES
against the mutation, because two counters only collide where their ranges
overlap and a fresh one sits far below the shared one. The discriminating
assertion is ORDERING -- an external id taken after a Thread object's id
must exceed it.

Four mutations, all caught.

#1958's own description is stale and is corrected rather than copied:
SR-AUD-214 and SR-AUD-189 were landed by #1971 on 2026-08-03, and SR-AUD-215
was deliberately excluded there with a measured reason. The ticket stays
blocked for SR-AUD-209 (vtable change), SR-AUD-194 (signature change),
SR-AUD-196 (removes public constructors) and SR-AUD-220 (storage on a public
template).

Downstream measured: 0 sites in cna, 0 in mobile-eggbert.

Gate: 17,420 run, 17,420 passed, 0 failed, 0 skipped across 38 executables
(+4 on 17,416; SharpRuntimeTests_Threading 471 -> 475; no other executable
moved).

Author: robertvokac

CLAUDE.md

@@ -0,0 +1,128 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — every thread now has a distinct `ManagedThreadId` (ticket #1958, SR-AUD-193)
+
+*2026-08-19.* `System::Threading::Thread::CurrentThread().getManagedThreadIdProperty()` returned
+**1** from every thread that was not created through a `System::Threading::Thread` object. The
+main thread reported 1, and so did every raw `std::thread`, every thread-pool worker, and every
+thread `std::async` created — all of them the same number, all of them colliding with the main
+thread and with each other.
+
+Landed under `docs/StandingApprovals.md` **SA-5** (aligning to the reference is ordinary work).
+**No public signature, object layout, vtable or `noexcept` change**, so consumers need no rebuild
+and no source edit. The id lives in a `thread_local`.
+
+---
+
+## 1. What changed
+
+| Caller | Was | Is |
+|---|---|---|
+| the main thread | `1` | `1` — unchanged, but now by **identity** (§3) |
+| a raw `std::thread` | `1` | a distinct id, `>= 2` |
+| a `std::async` / pool worker | `1` | a distinct id, `>= 2` |
+| the same thread, asked twice | `1`, `1` | the same id both times |
+| two different external threads | `1`, `1` | two different ids |
+| inside a `Thread` object's own body | that object's id | unchanged |
+| `someThread.getManagedThreadIdProperty()` | that object's id | unchanged |
+
+Anything that already reported a real id keeps reporting exactly the same one. The change is
+confined to threads that used to fall through to the hard-coded `1`.
+
+## 2. Why
+
+.NET's contract is stated in `Thread.cs` and is unconditional:
+
+> `ManagedThreadId` … "a unique identifier for the current managed thread"
+
+and its `ManagedThreadId` is read from the runtime's own per-thread block, which every thread has
+— managed, native-entered, or thread-pool. There is no path in .NET that returns a shared
+constant. A caller that keys a map, a lock-ownership record, a re-entrancy guard or a log column
+on this id was, in this port, keying all of them on the single value `1`.
+
+The old body said as much:
+
+```cpp
+return currentThreadState_ ? currentThreadState_->managedThreadId : 1;
+```
+
+`currentThreadState_` is only set by this port's own `Thread::Start`, so the `: 1` arm was every
+other thread in the process.
+
+## 3. The main thread keeps 1, and how it keeps it is the interesting part
+
+The naive repair — hand out `1` to whichever thread asks first, then `2, 3, …` — is wrong, and
+silently so. Nothing guarantees the main thread asks first; a worker started during static
+initialisation, or a logging thread that stamps its id before `main` runs, would take `1` and the
+main thread would become an ordinary numbered worker. The failure is intermittent, which is worse
+than consistent.
+
+The id is therefore assigned by **identity**:
+
+```cpp
+inline static const std::thread::id mainThreadOsId_ = std::this_thread::get_id();
+```
+
+That initialiser runs during static initialisation, which runs on the main thread, so
+`mainThreadOsId_` is the main thread's OS id no matter who asks first afterwards.
+
+## 4. One counter, not two
+
+External threads draw from `nextManagedId_` — the **same** counter `Thread`'s constructor uses.
+That is not an implementation detail: uniqueness is across *all* threads, not within each kind. A
+separate counter for external threads would give each kind internally distinct ids and still let
+the two kinds collide with each other, which is the whole defect in a different costume.
+
+This is the mutation that survived the first three tests (§6) and needed a case of its own.
+
+## 5. Assigned on first use
+
+A thread that never reads the property costs nothing — no counter increment, no allocation. The
+`thread_local` starts at `0`, which is not a legal managed id, and is filled in on the first
+call. So the id sequence reflects the order in which threads *asked*, not the order in which they
+started; .NET's does the same, and no documented behaviour depends on the numeric order.
+
+## 6. Evidence
+
+Four mutations, **all caught**:
+
+| Mutation | Caught by |
+|---|---|
+| M1 — external threads report `1` again | `Fix1958_193_ExternalThreadsGetDistinctIds` |
+| M2 — the id is reassigned on every call | `Fix1958_193_TheIdIsStableWithinAThread` |
+| M3 — the main thread loses its `1` | `Fix1958_193_ExternalThreadsGetDistinctIds` |
+| M4 — external ids come from a **separate** counter | `Fix1958_193_ExternalAndWrapperIdsShareOneCounter` |
+
+**M4 is worth recording in full, because the obvious test for it does not work.** Collecting four
+wrapper ids and four external ids and asserting the set has eight members *passes* against the
+mutation: two counters only collide where their ranges overlap, and in a test binary that has
+already created dozens of threads a freshly-started second counter sits far below the shared one,
+so the ids come out distinct — for the wrong reason. The assertion that discriminates is
+**ordering**: take a `Thread` object's id, then an external id assigned after it, and require the
+external one to be larger. That holds under one counter and fails under two, on the first run,
+with no dependence on how many threads ran before.
+
+Gate: **17,420 run, 17,420 passed, 0 failed, 0 skipped** across 38 executables — `+4` on 17,416,
+exactly the four cases added to `SharpRuntimeTests_Threading` (471 → 475). No other executable's
+count moved.
+
+## 7. Downstream, measured
+
+Per SA-2 condition 5 (recorded here although SA-2 is not the approval this landed under, since a
+behaviour change deserves the same measurement): `getManagedThreadIdProperty` appears in **zero**
+places in `cna` and **zero** in `mobile-eggbert`. Neither repository was modified.
+
+Any future consumer that used the id as a *thread-kind* discriminator — treating `1` as "not one
+of ours" — must stop; that reading was never what the property meant, and it no longer works.
+
+## 8. What #1958 still covers
+
+Ticket #1958 remains **blocked** for its other members, each of which needs an approval this one
+did not: SR-AUD-209 is a vtable/base-class change, SR-AUD-194 a public signature change,
+SR-AUD-196 removes public constructors, and SR-AUD-220 needs storage on a public template.
+
+Two members listed in #1958's description are **already closed and the description is stale**:
+SR-AUD-214 and SR-AUD-189 were both landed by ticket #1971 on 2026-08-03. SR-AUD-215 was
+deliberately excluded by #1971 with a measured reason — `Capture()` returns `nullptr`
+unconditionally, so rejecting null would break every reachable call.

docs/Migration-ThreadManagedThreadIdUniqueness.md

@@ -66,6 +66,28 @@ namespace System::Threading {
         // Thread object itself.
         inline static thread_local std::shared_ptr<RunState> currentThreadState_;
 
+        // #1958 / SR-AUD-193. A thread NOT started through this class used to report managed id
+        // 1 -- the main thread's -- so every external thread collided with it and with each
+        // other, erasing the uniqueness .NET guarantees. Each OS thread now takes an id from the
+        // same counter the wrapper uses, so no two threads can report the same one.
+        //
+        // The main thread keeps 1 by IDENTITY rather than by arriving first: its OS id is
+        // captured during static initialisation, which runs on it. Assigning "1 to whoever asks
+        // first" would hand it to a worker whenever a worker happens to ask before main does.
+        inline static const std::thread::id mainThreadOsId_ = std::this_thread::get_id();
+        inline static thread_local intcs    currentExternalId_ = 0;
+
+        /** @brief The managed id of the calling thread, assigned on first use. */
+        static intcs currentManagedThreadId() {
+            if (currentThreadState_) return currentThreadState_->managedThreadId;
+            if (currentExternalId_ == 0) {
+                currentExternalId_ = (std::this_thread::get_id() == mainThreadOsId_)
+                                         ? 1
+                                         : nextManagedId_.fetch_add(1);
+            }
+            return currentExternalId_;
+        }
+
         std::shared_ptr<RunState> state_ = std::make_shared<RunState>();
         std::function<void()>  fn_;
         std::thread            thread_;
@@ -315,13 +337,20 @@ namespace System::Threading {
             /**
              * @brief Returns the managed thread ID of the calling thread.
              *
-             * Resolves to the same ManagedThreadId the owning Thread object reports, when the
-             * calling thread was started via Thread::Start(); otherwise (the main thread, or
-             * any thread not created through this class) returns 1, matching .NET's
-             * main-thread convention.
+             * Resolves to the same ManagedThreadId the owning Thread object reports when the
+             * calling thread was started via Thread::Start(). Ticket #1958 / SR-AUD-193: a
+             * thread NOT created through this class used to return 1 unconditionally -- the main
+             * thread's id -- so every external thread collided with it and with every other
+             * external thread. Each now takes a distinct id from the same counter, which is the
+             * uniqueness .NET's ManagedThreadId guarantees.
+             *
+             * The main thread still reports 1, and now does so by identity rather than by
+             * arriving first: its OS thread id is captured during static initialisation.
+             *
+             * The id is assigned on FIRST USE, so a thread that never asks costs nothing.
              */
             [[nodiscard]] intcs getManagedThreadIdProperty() const {
-                return currentThreadState_ ? currentThreadState_->managedThreadId : 1;
+                return currentManagedThreadId();
             }
             /** @brief Returns whether the calling thread's owning Thread object is marked background. */
             [[nodiscard]] bool getIsBackgroundProperty() const {

modules/threading/include/System/Threading/Thread.hpp

@@ -9,6 +9,10 @@
 // ManualResetEventSlim, ReaderWriterLockSlim, SpinWait, ThreadLocal, ThreadPool, Timer,
 // and Threading exceptions.
 #include <gtest/gtest.h>
+#include "System/Threading/Thread.hpp"
+#include <vector>
+#include <memory>
+#include <set>
 #include <atomic>
 #include <chrono>
 #include <future>
@@ -1033,3 +1037,119 @@ THREADING_EXCEPT_SIMPLE(WaitHandleCannotBeOpenedException)
 TEST(WaitHandleCannotBeOpenedExceptionTests, IsA_ApplicationException) {
     EXPECT_THROW(throw WaitHandleCannotBeOpenedException(), System::ApplicationException);
 }
+
+// ===========================================================================
+// #1958 / SR-AUD-193 -- CurrentThread().ManagedThreadId is unique per thread.
+//
+// #1958 lists eight members; this is the one whose repair needs no approval
+// boundary -- no signature, no layout, no vtable, and the id lives in a
+// thread_local rather than in any public type. The other seven keep theirs, and
+// two of the eight (SR-AUD-214, SR-AUD-189) were already landed by #1971, which
+// the ticket's own description still describes as live.
+// ===========================================================================
+
+TEST(ThreadManagedIdTests, Fix1958_193_ExternalThreadsGetDistinctIds) {
+    // The defect: every thread not started through Thread reported 1 -- the main thread's id --
+    // so they collided with it and with each other.
+    const SharpRuntime::intcs mainId = System::Threading::Thread::CurrentThread().getManagedThreadIdProperty();
+    EXPECT_EQ(mainId, 1) << "the main thread keeps .NET's conventional id";
+
+    constexpr int kThreads = 8;
+    std::vector<SharpRuntime::intcs> ids(kThreads, 0);
+    std::vector<std::thread> workers;
+    for (int i = 0; i < kThreads; ++i) {
+        workers.emplace_back([&ids, i] {
+            // std::thread, NOT System::Threading::Thread -- these are exactly the "externally
+            // created" threads the finding is about.
+            ids[static_cast<size_t>(i)] =
+                System::Threading::Thread::CurrentThread().getManagedThreadIdProperty();
+        });
+    }
+    for (auto& w : workers) w.join();
+
+    std::set<SharpRuntime::intcs> distinct(ids.begin(), ids.end());
+    EXPECT_EQ(distinct.size(), static_cast<size_t>(kThreads)) << "ids collided";
+    EXPECT_EQ(distinct.count(mainId), 0u) << "an external thread took the main thread's id";
+    for (auto id : ids) EXPECT_GT(id, 1) << "an external thread reported the main-thread id";
+}
+
+TEST(ThreadManagedIdTests, Fix1958_193_TheIdIsStableWithinAThread) {
+    // Assigned on FIRST USE and then kept: two reads on one thread must agree, or the id is not
+    // an identity at all. The row that fails if the counter is bumped on every call.
+    SharpRuntime::intcs a = 0, b = 0;
+    std::thread worker([&a, &b] {
+        a = System::Threading::Thread::CurrentThread().getManagedThreadIdProperty();
+        b = System::Threading::Thread::CurrentThread().getManagedThreadIdProperty();
+    });
+    worker.join();
+    EXPECT_NE(a, 0);
+    EXPECT_EQ(a, b);
+
+    // And the main thread's is stable too.
+    EXPECT_EQ(System::Threading::Thread::CurrentThread().getManagedThreadIdProperty(),
+              System::Threading::Thread::CurrentThread().getManagedThreadIdProperty());
+}
+
+TEST(ThreadManagedIdTests, Fix1958_193_ExternalAndWrapperIdsShareOneCounter) {
+    // UNIQUENESS IS ACROSS ALL THREADS, not within each kind. A separate counter for external
+    // threads would give each kind distinct ids and still let the two kinds collide -- which is
+    // exactly the mutation this case was added for, after it went uncaught with only the
+    // per-kind tests above.
+    // Thread is non-copyable (one owned OS handle per object), so the objects are held by
+    // pointer rather than by value.
+    std::vector<SharpRuntime::intcs> wrapperIds;
+    std::vector<std::unique_ptr<System::Threading::Thread>> wrappers;
+    for (int i = 0; i < 4; ++i) {
+        wrappers.push_back(std::make_unique<System::Threading::Thread>([] {}));
+        wrapperIds.push_back(wrappers.back()->getManagedThreadIdProperty());
+    }
+
+    std::vector<SharpRuntime::intcs> externalIds(4, 0);
+    std::vector<std::thread> workers;
+    for (int i = 0; i < 4; ++i)
+        workers.emplace_back([&externalIds, i] {
+            externalIds[static_cast<size_t>(i)] =
+                System::Threading::Thread::CurrentThread().getManagedThreadIdProperty();
+        });
+    for (auto& w : workers) w.join();
+
+    std::set<SharpRuntime::intcs> all(wrapperIds.begin(), wrapperIds.end());
+    for (auto id : externalIds) {
+        EXPECT_EQ(all.count(id), 0u)
+            << "external id " << id << " collided with a Thread object's id";
+        all.insert(id);
+    }
+    EXPECT_EQ(all.size(), 8u);
+
+    // THE ASSERTION THAT ACTUALLY DISCRIMINATES A SECOND COUNTER, and it was added after the
+    // set-distinctness above failed to: two counters only collide where their ranges overlap, and
+    // in a binary that has already created many threads a fresh second counter sits far below
+    // the shared one, so the ids come out distinct for the wrong reason.
+    //
+    // Ordering is the invariant a single counter really provides: an id handed out LATER is
+    // larger. Take a wrapper id, then an external one after it, and the external must exceed it.
+    System::Threading::Thread marker([] {});
+    const SharpRuntime::intcs wrapperId = marker.getManagedThreadIdProperty();
+    SharpRuntime::intcs afterId = 0;
+    std::thread later([&afterId] {
+        afterId = System::Threading::Thread::CurrentThread().getManagedThreadIdProperty();
+    });
+    later.join();
+    EXPECT_GT(afterId, wrapperId)
+        << "an external id assigned after a Thread object's id was smaller, so the two kinds are "
+           "not sharing one counter";
+}
+
+TEST(ThreadManagedIdTests, Fix1958_193_AWrapperThreadStillReportsItsOwnObjectsId) {
+    // Unchanged behaviour, and the control: a thread started through Thread reports the id its
+    // Thread object was given at construction, not a freshly assigned external one.
+    SharpRuntime::intcs seenInside = 0;
+    System::Threading::Thread t([&seenInside] {
+        seenInside = System::Threading::Thread::CurrentThread().getManagedThreadIdProperty();
+    });
+    const SharpRuntime::intcs objectId = t.getManagedThreadIdProperty();
+    t.Start();
+    t.Join();
+    EXPECT_EQ(seenInside, objectId);
+    EXPECT_GT(objectId, 1);
+}