feat(threading): Thread::Start(void*) forwards its parameter (#1958, SR-AUD-194)

Rule-14 sweep. Start(void*) captured its argument and discarded it with a
literal (void)parameter; while its own doc-comment said the value was
"forwarded to the thread function" -- no way to receive it, no diagnostic,
because the only accepted callback shape had no parameter slot.

THE RECORDED COST ESTIMATE WAS WRONG. #1958 listed this as a public signature
change, routing it through SA-10 and SA-2. No existing signature changes: the
repair is an additive second constructor -- .NET's
Thread(ParameterizedThreadStart) (Thread.cs:152) -- plus an SA-5 behaviour
change. It therefore landed as ordinary work with a pinned layout. The one
spelling that would become ambiguous, Thread(nullptr), exists in zero places
across modules/, test/ and both consumers.

No shape flag was needed: exactly one of the two callables is ever set, and
which one IS .NET's `startHelper._start is ThreadStart` test.

sizeof(Thread) 104 -> 136; consumers rebuild.

Two asymmetries are .NET's and both are pinned: Start() does NOT reject a
parameterized thread (it passes null), and the shape check applies only
before the first start, because .NET wraps it in `if (startHelper != null)`
and says so in a comment. That second pin was written asserting the opposite
and FAILED -- the reference showed the test was wrong rather than the code.

Five mutations, all caught. Two only after repair: M1 was a SEGV because the
test dereferenced a null the mutation supplies, and M3 was a SIGABRT because
my mutation threw from a thread body, which is not a realistic regression.
M2 is caught only as a crash and that is inherent -- removing the guard hands
an empty std::function to a new thread and reaches std::terminate with no
handler, which no assertion can observe (#2215).

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

#1958 now has two findings left: SR-AUD-209 and SR-AUD-196.

Gate: 17,497 run, 17,497 passed, 0 failed, 0 skipped across 38 executables
(+7 on 17,490; SharpRuntimeTests_Threading 514 -> 521; no other executable
moved). Module graph unchanged at 41/93.

Author: robertvokac

CLAUDE.md

@@ -0,0 +1,118 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — `Thread::Start(void*)` forwards its parameter (ticket #1958, SR-AUD-194)
+
+*2026-08-19.* `System::Threading::Thread` gains a constructor taking
+`std::function<void(void*)>`, and `Start(void*)` now delivers its argument to it — or throws if
+the thread was built with the parameterless shape.
+
+**`sizeof(Thread)` grows 104 → 136, so consumers must be recompiled.** Landed under **SA-5** with
+**SA-3**'s layout condition discharged.
+
+---
+
+## 1. What was wrong
+
+`Start(void*)` captured its argument and then discarded it with a literal `(void)parameter;`,
+while the member's own doc-comment said the value was *"forwarded to the thread function"*. There
+was no way for a caller to receive it and no diagnostic saying so — because the only accepted
+callback shape had **no parameter slot**.
+
+## 2. The recorded cost estimate was wrong
+
+#1958's record listed SR-AUD-194 as *"a signature change"*, which would route it through SA-10
+and SA-2's five conditions. **Measured, no existing signature changes.** The repair is:
+
+* **additive** — a second constructor, the counterpart of .NET's
+  `Thread(ParameterizedThreadStart start)` (`Thread.cs:152`);
+* **a behaviour change** to `Start(void*)`, which is SA-5's ordinary territory.
+
+So it landed as ordinary work with a pinned layout, not as a source break. Nothing that compiled
+before compiles differently: a no-argument lambda converts only to `std::function<void()>`, a
+`void*`-taking lambda only to `std::function<void(void*)>`. The one spelling that would become
+ambiguous is `Thread(nullptr)`, and it exists in **zero** places across `modules/`, `test/`, `cna`
+and `mobile-eggbert`.
+
+## 3. What changes
+
+| | Was | Is |
+|---|---|---|
+| `Thread(std::function<void(void*)>)` | **absent** | present; empty callable → `ArgumentNullException` |
+| `Start(p)` on a **parameterized** thread | — | the body receives `p` |
+| `Start(p)` on a **parameterless** thread | silently ignored `p`, ran the body | `InvalidOperationException` |
+| `Start()` on a **parameterized** thread | — | runs with `nullptr`, **no exception** (§4) |
+| `Start()` on a parameterless thread | unchanged | unchanged |
+| `sizeof(Thread)` | **104** | **136** |
+
+The message is .NET's verbatim: *"The thread was created with a ThreadStart delegate that does not
+accept a parameter."*
+
+No separate shape flag was needed — exactly one of the two callables is ever set, and which one
+**is** the shape record. That is precisely .NET's `startHelper._start is ThreadStart` test.
+
+## 4. Two asymmetries, both .NET's, both pinned
+
+**`Start()` does not reject a parameterized thread.** .NET's private `Start(bool)` sets
+`startHelper._startArg = null` and performs **no** delegate-shape check at all
+(`Thread.cs:239-253`) — only `Start(parameter)` guards. "Reject it for symmetry" is the plausible
+wrong answer, so a test pins the permissive behaviour.
+
+**The shape check applies only before the first start.** .NET wraps it in
+`if (startHelper != null)` (`Thread.cs:204-214`), and the comment above says why: *"In the case of
+a null startHelper (second call to start on same thread) StartCore method will take care of the
+error reporting."* So a **second** `Start(void*)` reports the **restart** error, not the
+wrong-shape error.
+
+**That second pin was written asserting the opposite, and it failed.** The reference then showed
+that the *test* was wrong rather than the code: this port gets .NET's rule from the same fact,
+because `fn_` is moved from on the first successful start and is empty afterwards, exactly as
+.NET's `startHelper` becomes null. The test now asserts both halves — shape error before the
+first start, restart error after it.
+
+## 5. Evidence
+
+Five mutations, all caught:
+
+| Mutation | Caught by |
+|---|---|
+| M1 — the parameter is discarded again | `Fix1958_TheParameterActuallyReachesTheBody` — **by name, after repair** |
+| M2 — the shape guard is removed | **as a crash only** — see below |
+| M3 — `Start()` rejects a parameterized thread | `Decl1958_ParameterlessStartOnAParameterizedThreadPassesNull` — **by name, after reformulation** |
+| M4 — an empty parameterized callable is accepted | `Fix1958_AnEmptyParameterizedCallableIsRejectedAtConstruction` |
+| M5 — the new constructor consumes no managed id | `Fix1958_BothShapesGetDistinctManagedThreadIds` |
+
+M5 matters because SR-AUD-193's uniqueness contract covers **every** `Thread` object regardless of
+shape.
+
+**M1 was caught only as a SEGV at first**, because the test dereferenced the pointer and the
+mutation passes `nullptr`. The assertion is now null-safe, so it fails by name.
+
+**M3 was caught only as a SIGABRT at first**, because my mutation threw from inside the *thread
+body*, where no handler exists. That is not a realistic regression; reformulated as the plausible
+one — rejecting at the call site "for symmetry" — it is caught by name.
+
+**M2 is caught only as a crash, and that is inherent rather than a test defect.** Removing the
+guard lets `Start(void*)` on a parameterless thread hand an **empty** `std::function` to a new OS
+thread, whose call to it raises `std::bad_function_call` with no handler and reaches
+`std::terminate` — exactly the failure mode SR-AUD-192 documented for the constructor. The
+`EXPECT_THROW` does record its failure first, but the process aborts before gtest prints its
+summary. No assertion can observe a `std::terminate` on another thread, which is the same
+inherent limit #2215 recorded.
+
+Gate: **17,497 run, 17,497 passed, 0 failed, 0 skipped** across 38 executables — `+7` on 17,490,
+exactly the seven new cases (`SharpRuntimeTests_Threading` 514 → 521). No other executable moved.
+All 514 pre-existing cases passed unchanged before the new ones were added. Module graph
+unchanged at 41/93.
+
+## 6. Downstream, measured
+
+`System::Threading::Thread` appears in **zero** places in `cna` and **zero** in `mobile-eggbert`,
+so the rebuild requirement is recorded here for future consumers rather than acted on. Neither
+repository was modified.
+
+## 7. Scope
+
+#1958 now has **two** findings left: **SR-AUD-209** (make `AutoResetEvent`/`ManualResetEvent`
+derive from `WaitHandle` — a vtable *and* base-class change, which SA-3 explicitly excludes) and
+**SR-AUD-196** (`ThreadStartException` publishes constructors .NET makes internal).

docs/Migration-ThreadParameterizedStart.md

@@ -16,6 +16,7 @@
 #include "System/LocalDataStoreSlot.hpp"
 #include "SharpRuntime/SharpRuntimeHelper.hpp"
 #include "System/ArgumentNullException.hpp"
+#include "System/InvalidOperationException.hpp"
 #include "System/ArgumentOutOfRangeException.hpp"
 #include "System/Threading/ApartmentState.hpp"
 #include "System/Threading/ThreadPriority.hpp"
@@ -90,6 +91,11 @@ namespace System::Threading {
 
         std::shared_ptr<RunState> state_ = std::make_shared<RunState>();
         std::function<void()>  fn_;
+        // Ticket #1958 / SR-AUD-194. Exactly ONE of these two is ever set, and which one records
+        // the delegate shape -- so no separate flag is needed. `fn_` non-empty means the thread
+        // was built with the parameterless shape, which is precisely .NET's
+        // `startHelper._start is ThreadStart` test.
+        std::function<void(void*)> paramFn_;
         std::thread            thread_;
         std::string            name_;
         bool                   isThreadPoolThread_ = false;
@@ -120,6 +126,31 @@ namespace System::Threading {
             state_->managedThreadId = nextManagedId_.fetch_add(1);
         }
 
+        /**
+         * @brief Constructs a Thread whose body ACCEPTS the parameter `Start(void*)` supplies.
+         * @param start Function to execute on the new thread, receiving `Start`'s argument.
+         * @throws System::ArgumentNullException if @p start is an empty std::function.
+         *
+         * C++ counterpart of .NET's `Thread(ParameterizedThreadStart start)`
+         * (`Thread.cs:152`). Ticket #1958 / SR-AUD-194.
+         *
+         * **This constructor is what makes `Start(void*)` mean anything.** Before it, the only
+         * accepted callback shape had no parameter slot, so `Start(void*)` captured its argument
+         * and then discarded it with a literal `(void)parameter;` while its own doc-comment said
+         * the value was "forwarded to the thread function". There was no way for a caller to
+         * receive it and no diagnostic saying so.
+         *
+         * The same `ArgumentNullException` guard applies for the same reason as the parameterless
+         * constructor's (SR-AUD-192): deferring it means `std::bad_function_call` on a thread with
+         * no handler, i.e. `std::terminate`.
+         */
+        explicit Thread(std::function<void(void*)> start)
+            : paramFn_(std::move(start))
+        {
+            if (!paramFn_) throw System::ArgumentNullException("start");
+            state_->managedThreadId = nextManagedId_.fetch_add(1);
+        }
+
         ~Thread() {
             if (thread_.joinable()) thread_.detach();
         }
@@ -138,9 +169,15 @@ namespace System::Threading {
         void Start() {
             if (started_.exchange(true))
                 throw System::Threading::ThreadStateException("Thread is running or terminated; it cannot restart.");
-            thread_ = std::thread([state = state_, fn = std::move(fn_)]() mutable {
+            // A PARAMETERIZED thread started this way runs with a null argument and NO exception.
+            // That asymmetry is .NET's: its private Start(bool) sets `startHelper._startArg = null`
+            // and performs no delegate-shape check at all (Thread.cs:239-253) -- only
+            // Start(parameter) guards. Pinned by a test, because "reject it for symmetry" is the
+            // plausible wrong answer.
+            thread_ = std::thread([state = state_, fn = std::move(fn_),
+                                   paramFn = std::move(paramFn_)]() mutable {
                 currentThreadState_ = state;
-                fn();
+                if (fn) fn(); else paramFn(nullptr);
                 state->finished.store(true);
             });
         }
@@ -151,12 +188,29 @@ namespace System::Threading {
          * @throws System::Threading::ThreadStateException if Start() has already been called.
          */
         void Start(void* parameter) {
+            // THE SHAPE CHECK COMES FIRST -- but only while the thread has NOT been started, and
+            // that qualification is .NET's rather than an accident here. Its private
+            // Start(object, bool) wraps the whole check in `if (startHelper != null)`
+            // (Thread.cs:204-214), and the comment two lines above says why: "In the case of a
+            // null startHelper (second call to start on same thread) StartCore method will take
+            // care of the error reporting."
+            //
+            // So a SECOND Start(void*) reports the RESTART error, not the wrong-shape error. This
+            // port gets the same rule from the same fact: fn_ is MOVED FROM into the thread body
+            // on the first successful start, so it is empty afterwards and the guard falls
+            // through -- exactly as .NET's startHelper becomes null. A pin asserts both halves,
+            // and it was written asserting the opposite first: the test failed, and the reference
+            // showed the test was wrong rather than the code.
+            if (fn_) {
+                throw System::InvalidOperationException(
+                    "The thread was created with a ThreadStart delegate that does not accept a "
+                    "parameter.");
+            }
             if (started_.exchange(true))
                 throw System::Threading::ThreadStateException("Thread is running or terminated; it cannot restart.");
-            thread_ = std::thread([state = state_, fn = std::move(fn_), parameter]() mutable {
+            thread_ = std::thread([state = state_, paramFn = std::move(paramFn_), parameter]() mutable {
                 currentThreadState_ = state;
-                (void)parameter;
-                fn();
+                paramFn(parameter);
                 state->finished.store(true);
             });
         }

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

@@ -1587,3 +1587,112 @@ TEST(ThreadLocalValuesTests, Fix1958_DisposeReleasesTheTrackedValues) {
 
     EXPECT_THROW((void)local.getValuesProperty(), System::ObjectDisposedException);
 }
+
+// =============================================================================================
+// Ticket #1958 / SR-AUD-194 — Thread::Start(void*) actually forwards its parameter.
+//
+// The member captured its argument and then discarded it with a literal `(void)parameter;`,
+// while its own doc-comment said the value was "forwarded to the thread function". There was no
+// way for a caller to receive it and no diagnostic saying so, because the ONLY accepted callback
+// shape had no parameter slot.
+//
+// The repair is therefore ADDITIVE, not a signature change: a second constructor taking
+// std::function<void(void*)> -- .NET's Thread(ParameterizedThreadStart) (Thread.cs:152) -- plus a
+// guard on Start(void*) matching .NET's `startHelper._start is ThreadStart` test
+// (Thread.cs:206-210), with .NET's verbatim message.
+//
+// Landed under SA-5 with SA-3's layout condition discharged: sizeof(Thread) grows 104 -> 136.
+// =============================================================================================
+
+TEST(ThreadParameterizedStartTests, Decl1958_TheSecondCallableGrowsTheType) {
+    static_assert(sizeof(System::Threading::Thread) == 136,
+                  "#1958/SR-AUD-194 grew Thread 104 -> 136; a further change needs its own pin");
+    EXPECT_EQ(sizeof(System::Threading::Thread), 136u);
+}
+
+TEST(ThreadParameterizedStartTests, Fix1958_TheParameterActuallyReachesTheBody) {
+    // THE DEFECT: before the repair there was no callable shape that could receive this at all.
+    int payload = 4242;
+    std::atomic<int> seen{0};
+    // Null-SAFE on purpose: the mutation that discards the parameter again passes nullptr, and a
+    // dereference there would SEGV -- caught, but as a crash rather than by name.
+    System::Threading::Thread t(std::function<void(void*)>(
+        [&seen](void* p) { seen.store(p ? *static_cast<int*>(p) : -1); }));
+    t.Start(&payload);
+    t.Join();
+    EXPECT_EQ(seen.load(), 4242);
+}
+
+TEST(ThreadParameterizedStartTests, Fix1958_StartWithAParameterOnAParameterlessThreadThrows) {
+    // .NET: `if (startHelper._start is ThreadStart) throw new InvalidOperationException(
+    //        SR.InvalidOperation_ThreadWrongThreadStart);` (Thread.cs:206-210).
+    std::atomic<bool> ran{false};
+    System::Threading::Thread t(std::function<void()>([&ran] { ran.store(true); }));
+    int payload = 1;
+    EXPECT_THROW(t.Start(&payload), System::InvalidOperationException);
+    EXPECT_FALSE(ran.load()) << "the rejected Start must not have started anything";
+
+    // ...and the thread is still usable through the door that matches its shape.
+    EXPECT_NO_THROW(t.Start());
+    t.Join();
+    EXPECT_TRUE(ran.load());
+}
+
+TEST(ThreadParameterizedStartTests, Decl1958_ParameterlessStartOnAParameterizedThreadPassesNull) {
+    // THE ASYMMETRY, pinned because "reject it for symmetry" is the plausible wrong answer.
+    // .NET's private Start(bool) sets `startHelper._startArg = null` and performs NO delegate
+    // shape check at all (Thread.cs:239-253) -- only Start(parameter) guards.
+    std::atomic<bool> ran{false};
+    std::atomic<bool> gotNull{false};
+    System::Threading::Thread t(std::function<void(void*)>([&](void* p) {
+        gotNull.store(p == nullptr);
+        ran.store(true);
+    }));
+    EXPECT_NO_THROW(t.Start()) << "a parameterized thread may be started with no parameter";
+    t.Join();
+    EXPECT_TRUE(ran.load());
+    EXPECT_TRUE(gotNull.load()) << "it receives null, as .NET's _startArg = null gives";
+}
+
+TEST(ThreadParameterizedStartTests, Decl1958_TheShapeCheckAppliesOnlyBeforeTheFirstStart) {
+    // THIS TEST WAS WRITTEN ASSERTING THE OPPOSITE AND FAILED -- and the reference showed the
+    // TEST was wrong, not the code. .NET wraps the whole shape check in
+    // `if (startHelper != null)` (Thread.cs:204-214), and the comment above it says why: "In the
+    // case of a null startHelper (second call to start on same thread) StartCore method will take
+    // care of the error reporting."
+    //
+    // So the wrong-shape error is reported only while the thread has not been started; a SECOND
+    // Start(void*) reports the RESTART error. This port gets the same rule from the same fact --
+    // fn_ is moved from on the first successful start, exactly as .NET's startHelper is nulled.
+    std::atomic<bool> ran{false};
+    System::Threading::Thread t(std::function<void()>([&ran] { ran.store(true); }));
+
+    // Before any start: the shape check fires.
+    int payload = 1;
+    EXPECT_THROW(t.Start(&payload), System::InvalidOperationException);
+
+    t.Start();
+    t.Join();
+
+    // After starting: the restart check fires, through EITHER door.
+    EXPECT_THROW(t.Start(&payload), System::Threading::ThreadStateException)
+        << "a second Start(parameter) reports the restart, as .NET's null startHelper does";
+    EXPECT_THROW(t.Start(), System::Threading::ThreadStateException);
+}
+
+TEST(ThreadParameterizedStartTests, Fix1958_AnEmptyParameterizedCallableIsRejectedAtConstruction) {
+    // The same SR-AUD-192 reasoning as the parameterless constructor: deferring it means
+    // std::bad_function_call on a thread with no handler, i.e. std::terminate.
+    EXPECT_THROW(System::Threading::Thread(std::function<void(void*)>()),
+                 System::ArgumentNullException);
+}
+
+TEST(ThreadParameterizedStartTests, Fix1958_BothShapesGetDistinctManagedThreadIds) {
+    // The new constructor must consume an id like the old one -- SR-AUD-193's uniqueness contract
+    // covers every Thread object regardless of shape.
+    System::Threading::Thread a(std::function<void()>([] {}));
+    System::Threading::Thread b(std::function<void(void*)>([](void*) {}));
+    EXPECT_NE(a.getManagedThreadIdProperty(), b.getManagedThreadIdProperty());
+    EXPECT_GT(a.getManagedThreadIdProperty(), 1);
+    EXPECT_GT(b.getManagedThreadIdProperty(), 1);
+}