From 00a8291864ca13b02663928c77bb0f40c4cb634a Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Thu, 2 Jul 2026 22:56:54 -0700 Subject: [PATCH 1/5] Fix NullReferenceException for null optional non-blittable struct params Optional [In] pointer parameters to non-blittable (managed) structs were emitted with an `in` modifier because a pointer to a managed type is illegal. The friendly overload then passed `ref Unsafe.NullRef()` on the null path, which caused the P/Invoke marshaler to dereference a null reference and throw a NullReferenceException (e.g. MiniDumpWriteDump(..., CallbackParam: null)). Emit such parameters as a `T[]` array instead, as is already done for non-blittable struct fields. A null array marshals to a null pointer and a single-element array marshals to a pointer to the struct. The friendly overload continues to expose the parameter as a nullable `T?`. Fixes #1739 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Generator.FriendlyOverloads.cs | 39 ++++++--- .../PointerTypeHandleInfo.cs | 17 ++++ test/GenerationSandbox.Tests/MiniDumpTests.cs | 82 +++++++++++++++++++ .../GenerationSandbox.Tests/NativeMethods.txt | 1 + 4 files changed, 128 insertions(+), 11 deletions(-) create mode 100644 test/GenerationSandbox.Tests/MiniDumpTests.cs diff --git a/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs b/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs index 5aa8b160..a2441225 100644 --- a/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs +++ b/src/Microsoft.Windows.CsWin32/Generator.FriendlyOverloads.cs @@ -904,17 +904,34 @@ private IEnumerable DeclareFriendlyOverload( signatureChanged = true; parameters[paramIndex] = parameters[paramIndex] .WithType(NullableType(elementType).WithTrailingTrivia(TriviaList(Space))); - leadingStatements.Add( - LocalDeclarationStatement(VariableDeclaration( - elementType, - [ - VariableDeclarator(localName.Identifier, EqualsValueClause( - BinaryExpression(SyntaxKind.CoalesceExpression, origName, DefaultExpression(elementType)))) - ]))); - arguments[paramIndex] = Argument(ConditionalExpression( - MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, origName, IdentifierName("HasValue")), - PrefixUnaryExpression(SyntaxKind.AddressOfExpression, localName), - LiteralExpression(SyntaxKind.NullLiteralExpression))); + if (externParam.Type is ArrayTypeSyntax) + { + // The extern method exposes this [In, Optional] managed (non-blittable) struct parameter as an + // array because a pointer to a managed type is illegal and an `in` modifier cannot represent a + // null pointer (passing null would make the marshaler dereference a null reference). Pass a + // single-element array when a value is provided, or null to omit it. A null array marshals to a + // null pointer. See https://github.com/microsoft/CsWin32/issues/1739. + arguments[paramIndex] = Argument(ConditionalExpression( + MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, origName, IdentifierName(nameof(Nullable.HasValue))), + ImplicitArrayCreationExpression(InitializerExpression( + SyntaxKind.ArrayInitializerExpression, + [MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, origName, IdentifierName(nameof(Nullable.Value)))])), + LiteralExpression(SyntaxKind.NullLiteralExpression))); + } + else + { + leadingStatements.Add( + LocalDeclarationStatement(VariableDeclaration( + elementType, + [ + VariableDeclarator(localName.Identifier, EqualsValueClause( + BinaryExpression(SyntaxKind.CoalesceExpression, origName, DefaultExpression(elementType)))) + ]))); + arguments[paramIndex] = Argument(ConditionalExpression( + MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, origName, IdentifierName("HasValue")), + PrefixUnaryExpression(SyntaxKind.AddressOfExpression, localName), + LiteralExpression(SyntaxKind.NullLiteralExpression))); + } } else if (isIn && isOut) { diff --git a/src/Microsoft.Windows.CsWin32/PointerTypeHandleInfo.cs b/src/Microsoft.Windows.CsWin32/PointerTypeHandleInfo.cs index f100c569..65806895 100644 --- a/src/Microsoft.Windows.CsWin32/PointerTypeHandleInfo.cs +++ b/src/Microsoft.Windows.CsWin32/PointerTypeHandleInfo.cs @@ -51,6 +51,23 @@ elementTypeDetails.MarshalUsingType is string || bool xIn = (parameterAttributes & ParameterAttributes.In) == ParameterAttributes.In; bool xOut = (parameterAttributes & ParameterAttributes.Out) == ParameterAttributes.Out; + // An optional [In] (but not [Out]) parameter that points to a managed (non-blittable) struct cannot be + // exposed as a pointer (pointers to managed types are illegal) and an `in` modifier cannot represent a + // null pointer: passing null makes the marshaler dereference a null reference (NullReferenceException). + // Represent it as an array instead (as we already do for such struct fields below): a null array marshals + // to a null pointer and a single-element array marshals to a pointer to the struct. A friendly overload + // exposes this as a nullable value. See https://github.com/microsoft/CsWin32/issues/1739. + if (xIn && !xOut && xOptional && inputs.AllowMarshaling && nativeArrayInfo is null + && forElement == Generator.GeneratingElement.ExternMethod + && inputs.Generator?.IsManagedType(this.ElementType) is true + && this.ElementType.IsValueType(inputs) is true) + { + return new TypeSyntaxAndMarshaling( + ArrayType(elementTypeDetails.Type, [ArrayRankSpecifier()]), + elementTypeDetails.MarshalAsAttribute is object ? new MarshalAsAttribute(UnmanagedType.LPArray) { ArraySubType = elementTypeDetails.MarshalAsAttribute.Value } : null, + elementTypeDetails.NativeArrayInfo); + } + // A pointer to a marshaled object is not allowed. if (inputs.AllowMarshaling && customAttributes.HasValue && nativeArrayInfo is not null) { diff --git a/test/GenerationSandbox.Tests/MiniDumpTests.cs b/test/GenerationSandbox.Tests/MiniDumpTests.cs new file mode 100644 index 00000000..32880422 --- /dev/null +++ b/test/GenerationSandbox.Tests/MiniDumpTests.cs @@ -0,0 +1,82 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Diagnostics; +using System.Runtime.InteropServices; +using Windows.Win32; +using Windows.Win32.Foundation; +using Windows.Win32.System.Diagnostics.Debug; + +[Trait("WindowsOnly", "true")] +public class MiniDumpTests +{ + [Fact] + public void NullOptionalPointerParametersDoNotThrow() + { + // Regression test for https://github.com/microsoft/CsWin32/issues/1739. + // CallbackParam points at MINIDUMP_CALLBACK_INFORMATION, which is non-blittable + // (it contains a delegate field). Passing null for it must marshal to a null + // pointer rather than causing the marshaler to dereference a null reference. + using Process process = Process.GetCurrentProcess(); + string dumpPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + try + { + using FileStream dumpStream = File.Create(dumpPath); + BOOL result = PInvoke.MiniDumpWriteDump( + process.SafeHandle, + (uint)process.Id, + dumpStream.SafeFileHandle, + MINIDUMP_TYPE.MiniDumpNormal, + ExceptionParam: null, + UserStreamParam: null, + CallbackParam: null); + Assert.True(result, $"MiniDumpWriteDump failed with error 0x{Marshal.GetLastWin32Error():X}."); + } + finally + { + File.Delete(dumpPath); + } + } + + [Fact] + public unsafe void CallbackParameterIsMarshaled() + { + // A non-null CallbackParam is marshaled through a single-element array, which produces + // a pointer to the struct that the native function dereferences and calls back into. + // This verifies the array-based projection actually forwards the value (not just null). + bool callbackInvoked = false; + MINIDUMP_CALLBACK_ROUTINE callback = (void* param, MINIDUMP_CALLBACK_INPUT* input, MINIDUMP_CALLBACK_OUTPUT* output) => + { + callbackInvoked = true; + return true; + }; + + var callbackInfo = new MINIDUMP_CALLBACK_INFORMATION + { + CallbackRoutine = callback, + CallbackParam = null, + }; + + using Process process = Process.GetCurrentProcess(); + string dumpPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); + try + { + using FileStream dumpStream = File.Create(dumpPath); + PInvoke.MiniDumpWriteDump( + process.SafeHandle, + (uint)process.Id, + dumpStream.SafeFileHandle, + MINIDUMP_TYPE.MiniDumpNormal, + ExceptionParam: null, + UserStreamParam: null, + CallbackParam: callbackInfo); + } + finally + { + GC.KeepAlive(callback); + File.Delete(dumpPath); + } + + Assert.True(callbackInvoked); + } +} diff --git a/test/GenerationSandbox.Tests/NativeMethods.txt b/test/GenerationSandbox.Tests/NativeMethods.txt index d540f0bc..de2d9bc4 100644 --- a/test/GenerationSandbox.Tests/NativeMethods.txt +++ b/test/GenerationSandbox.Tests/NativeMethods.txt @@ -54,6 +54,7 @@ MAKELPARAM MAKELRESULT MAKEWPARAM MAX_PATH +MiniDumpWriteDump NTSTATUS PAGESET PathParseIconLocation From c20c1fd3289eab83166ed3ad36c41447899620d6 Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Fri, 3 Jul 2026 21:29:14 -0700 Subject: [PATCH 2/5] Replace deadlock-prone MiniDump callback test with a generator assertion The CallbackParameterIsMarshaled runtime test called MiniDumpWriteDump on the current process with a managed callback. Dumping your own process suspends all other threads while the callback runs on the calling thread, which can deadlock when the callback re-enters the CLR. It hung on net9/net10 in CI (blame-hang timeout). Verify the non-null array marshaling deterministically via a generator test instead, and keep the safe null-path runtime regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/GenerationSandbox.Tests/MiniDumpTests.cs | 48 +++---------------- .../GeneratorTests.cs | 29 +++++++++++ 2 files changed, 35 insertions(+), 42 deletions(-) diff --git a/test/GenerationSandbox.Tests/MiniDumpTests.cs b/test/GenerationSandbox.Tests/MiniDumpTests.cs index 32880422..e2464db7 100644 --- a/test/GenerationSandbox.Tests/MiniDumpTests.cs +++ b/test/GenerationSandbox.Tests/MiniDumpTests.cs @@ -17,6 +17,12 @@ public void NullOptionalPointerParametersDoNotThrow() // CallbackParam points at MINIDUMP_CALLBACK_INFORMATION, which is non-blittable // (it contains a delegate field). Passing null for it must marshal to a null // pointer rather than causing the marshaler to dereference a null reference. + // + // Note: this deliberately passes null for CallbackParam so that no managed callback + // runs while the process is being dumped. Supplying a callback here would let dbghelp + // invoke managed code while other threads are suspended for the dump, which can + // deadlock; the non-null marshaling is instead verified deterministically by the + // generator test MiniDumpWriteDump_OptionalNonBlittableStructMarshaledViaArray. using Process process = Process.GetCurrentProcess(); string dumpPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); try @@ -37,46 +43,4 @@ public void NullOptionalPointerParametersDoNotThrow() File.Delete(dumpPath); } } - - [Fact] - public unsafe void CallbackParameterIsMarshaled() - { - // A non-null CallbackParam is marshaled through a single-element array, which produces - // a pointer to the struct that the native function dereferences and calls back into. - // This verifies the array-based projection actually forwards the value (not just null). - bool callbackInvoked = false; - MINIDUMP_CALLBACK_ROUTINE callback = (void* param, MINIDUMP_CALLBACK_INPUT* input, MINIDUMP_CALLBACK_OUTPUT* output) => - { - callbackInvoked = true; - return true; - }; - - var callbackInfo = new MINIDUMP_CALLBACK_INFORMATION - { - CallbackRoutine = callback, - CallbackParam = null, - }; - - using Process process = Process.GetCurrentProcess(); - string dumpPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - try - { - using FileStream dumpStream = File.Create(dumpPath); - PInvoke.MiniDumpWriteDump( - process.SafeHandle, - (uint)process.Id, - dumpStream.SafeFileHandle, - MINIDUMP_TYPE.MiniDumpNormal, - ExceptionParam: null, - UserStreamParam: null, - CallbackParam: callbackInfo); - } - finally - { - GC.KeepAlive(callback); - File.Delete(dumpPath); - } - - Assert.True(callbackInvoked); - } } diff --git a/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs b/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs index b61934c3..63dbabd6 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs @@ -960,6 +960,35 @@ public void MiniDumpWriteDump_AllOptionalPointerParametersAreOptional(string tfm Assert.All(externMethod.ParameterList.Parameters.Reverse().Take(3), p => Assert.IsType(p.Type)); } + [Theory] + [MemberData(nameof(TFMDataNoNetFx35MemberData))] + public void MiniDumpWriteDump_OptionalNonBlittableStructMarshaledViaArray(string tfm) + { + // Regression test for https://github.com/microsoft/CsWin32/issues/1739. + // CallbackParam points at MINIDUMP_CALLBACK_INFORMATION, a non-blittable managed struct + // (it contains a delegate field). The extern cannot expose it as a pointer, so the friendly + // overload must forward it via a single-element array (or null): a null array marshals to a + // null pointer (no NullReferenceException) and a one-element array marshals to a pointer to + // the struct. + this.compilation = this.starterCompilations[tfm].WithOptions(this.compilation.Options.WithPlatform(Platform.X64)); + this.GenerateApi("MiniDumpWriteDump"); + + MethodDeclarationSyntax friendlyOverload = Assert.Single(this.FindGeneratedMethod("MiniDumpWriteDump"), m => !m.Modifiers.Any(SyntaxKind.ExternKeyword)); + + // CallbackParam is exposed as a nullable value... + ParameterSyntax callbackParam = Assert.Single(friendlyOverload.ParameterList.Parameters, p => p.Identifier.ValueText == "CallbackParam"); + Assert.IsType(callbackParam.Type); + + // ...and forwarded to the extern method as `CallbackParam.HasValue ? new[] { CallbackParam.Value } : null`. + InvocationExpressionSyntax externInvocation = Assert.Single( + friendlyOverload.DescendantNodes().OfType(), + i => i.Expression is MemberAccessExpressionSyntax { Name.Identifier.ValueText: "MiniDumpWriteDump" }); + ArgumentSyntax callbackArgument = Assert.Single(externInvocation.ArgumentList.Arguments, a => a.Expression is ConditionalExpressionSyntax { Condition: MemberAccessExpressionSyntax { Expression: IdentifierNameSyntax { Identifier.ValueText: "CallbackParam" } } }); + ConditionalExpressionSyntax conditional = Assert.IsType(callbackArgument.Expression); + Assert.IsType(conditional.WhenTrue); + Assert.True(conditional.WhenFalse.IsKind(SyntaxKind.NullLiteralExpression)); + } + [Fact] public void ContainsIllegalCharactersForAPIName_InvisibleCharacters() { From aaf8edfb2b673ecfc96bd80447f9177f9630f14d Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Fri, 3 Jul 2026 22:09:28 -0700 Subject: [PATCH 3/5] Replace generator signature test with a runtime threadpool interop test The non-null marshaling path for optional non-blittable struct parameters (issue #1739) is now verified by actually executing interop: TrySubmitThreadpoolCallback submits a callback with a non-null TP_CALLBACK_ENVIRON_V3 environment (non-blittable, has delegate fields) and asserts the callback runs. This exercises the single-element-array marshaling end to end instead of only inspecting the generated syntax. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/GenerationSandbox.Tests/MiniDumpTests.cs | 4 +- .../GenerationSandbox.Tests/NativeMethods.txt | 1 + .../ThreadpoolCallbackTests.cs | 40 +++++++++++++++++++ .../GeneratorTests.cs | 29 -------------- 4 files changed, 43 insertions(+), 31 deletions(-) create mode 100644 test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs diff --git a/test/GenerationSandbox.Tests/MiniDumpTests.cs b/test/GenerationSandbox.Tests/MiniDumpTests.cs index e2464db7..03972757 100644 --- a/test/GenerationSandbox.Tests/MiniDumpTests.cs +++ b/test/GenerationSandbox.Tests/MiniDumpTests.cs @@ -21,8 +21,8 @@ public void NullOptionalPointerParametersDoNotThrow() // Note: this deliberately passes null for CallbackParam so that no managed callback // runs while the process is being dumped. Supplying a callback here would let dbghelp // invoke managed code while other threads are suspended for the dump, which can - // deadlock; the non-null marshaling is instead verified deterministically by the - // generator test MiniDumpWriteDump_OptionalNonBlittableStructMarshaledViaArray. + // deadlock; the non-null marshaling is instead verified at runtime by + // ThreadpoolCallbackTests.NonNullOptionalNonBlittableStructIsMarshaled. using Process process = Process.GetCurrentProcess(); string dumpPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); try diff --git a/test/GenerationSandbox.Tests/NativeMethods.txt b/test/GenerationSandbox.Tests/NativeMethods.txt index de2d9bc4..19a8ee6c 100644 --- a/test/GenerationSandbox.Tests/NativeMethods.txt +++ b/test/GenerationSandbox.Tests/NativeMethods.txt @@ -83,6 +83,7 @@ ShellWindowTypeConstants SHFILEOPSTRUCTW SIZE SYSTEM_INFO +TrySubmitThreadpoolCallback VARDESC WER_REPORT_INFORMATION wglGetProcAddress diff --git a/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs b/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs new file mode 100644 index 00000000..48ff0847 --- /dev/null +++ b/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Runtime.InteropServices; +using Windows.Win32; +using Windows.Win32.Foundation; +using Windows.Win32.System.Threading; + +[Trait("WindowsOnly", "true")] +public class ThreadpoolCallbackTests +{ + [Fact] + public unsafe void NonNullOptionalNonBlittableStructIsMarshaled() + { + // Regression test for https://github.com/microsoft/CsWin32/issues/1739. + // TP_CALLBACK_ENVIRON_V3 is non-blittable (it contains delegate fields), so in the default + // marshaling mode CsWin32 exposes the optional pcbe parameter as a nullable value type and + // forwards a non-null value through a single-element array. This test exercises that array + // marshaling path end to end: the environment must reach the native threadpool intact for + // the submitted callback to actually run. + using ManualResetEventSlim callbackRan = new(false); + PTP_SIMPLE_CALLBACK callback = (instance, context) => callbackRan.Set(); + + TP_CALLBACK_ENVIRON_V3 environment = default; + environment.Version = 3; + environment.CallbackPriority = TP_CALLBACK_PRIORITY.TP_CALLBACK_PRIORITY_NORMAL; + environment.Size = (uint)Marshal.SizeOf(); + + try + { + BOOL submitted = PInvoke.TrySubmitThreadpoolCallback(callback, environment); + Assert.True(submitted, $"TrySubmitThreadpoolCallback failed with error 0x{Marshal.GetLastWin32Error():X}."); + Assert.True(callbackRan.Wait(TimeSpan.FromSeconds(30)), "The threadpool callback did not run."); + } + finally + { + GC.KeepAlive(callback); + } + } +} diff --git a/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs b/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs index 63dbabd6..b61934c3 100644 --- a/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs +++ b/test/Microsoft.Windows.CsWin32.Tests/GeneratorTests.cs @@ -960,35 +960,6 @@ public void MiniDumpWriteDump_AllOptionalPointerParametersAreOptional(string tfm Assert.All(externMethod.ParameterList.Parameters.Reverse().Take(3), p => Assert.IsType(p.Type)); } - [Theory] - [MemberData(nameof(TFMDataNoNetFx35MemberData))] - public void MiniDumpWriteDump_OptionalNonBlittableStructMarshaledViaArray(string tfm) - { - // Regression test for https://github.com/microsoft/CsWin32/issues/1739. - // CallbackParam points at MINIDUMP_CALLBACK_INFORMATION, a non-blittable managed struct - // (it contains a delegate field). The extern cannot expose it as a pointer, so the friendly - // overload must forward it via a single-element array (or null): a null array marshals to a - // null pointer (no NullReferenceException) and a one-element array marshals to a pointer to - // the struct. - this.compilation = this.starterCompilations[tfm].WithOptions(this.compilation.Options.WithPlatform(Platform.X64)); - this.GenerateApi("MiniDumpWriteDump"); - - MethodDeclarationSyntax friendlyOverload = Assert.Single(this.FindGeneratedMethod("MiniDumpWriteDump"), m => !m.Modifiers.Any(SyntaxKind.ExternKeyword)); - - // CallbackParam is exposed as a nullable value... - ParameterSyntax callbackParam = Assert.Single(friendlyOverload.ParameterList.Parameters, p => p.Identifier.ValueText == "CallbackParam"); - Assert.IsType(callbackParam.Type); - - // ...and forwarded to the extern method as `CallbackParam.HasValue ? new[] { CallbackParam.Value } : null`. - InvocationExpressionSyntax externInvocation = Assert.Single( - friendlyOverload.DescendantNodes().OfType(), - i => i.Expression is MemberAccessExpressionSyntax { Name.Identifier.ValueText: "MiniDumpWriteDump" }); - ArgumentSyntax callbackArgument = Assert.Single(externInvocation.ArgumentList.Arguments, a => a.Expression is ConditionalExpressionSyntax { Condition: MemberAccessExpressionSyntax { Expression: IdentifierNameSyntax { Identifier.ValueText: "CallbackParam" } } }); - ConditionalExpressionSyntax conditional = Assert.IsType(callbackArgument.Expression); - Assert.IsType(conditional.WhenTrue); - Assert.True(conditional.WhenFalse.IsKind(SyntaxKind.NullLiteralExpression)); - } - [Fact] public void ContainsIllegalCharactersForAPIName_InvisibleCharacters() { From 235f234dc627508138777a12e06a01f54f458399 Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Fri, 3 Jul 2026 22:19:22 -0700 Subject: [PATCH 4/5] Pass TestContext.Current.CancellationToken to satisfy xUnit1051 The Release build treats analyzer warnings as errors; ManualResetEventSlim.Wait must receive the test cancellation token. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs b/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs index 48ff0847..a19bd5e9 100644 --- a/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs +++ b/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs @@ -30,7 +30,7 @@ public unsafe void NonNullOptionalNonBlittableStructIsMarshaled() { BOOL submitted = PInvoke.TrySubmitThreadpoolCallback(callback, environment); Assert.True(submitted, $"TrySubmitThreadpoolCallback failed with error 0x{Marshal.GetLastWin32Error():X}."); - Assert.True(callbackRan.Wait(TimeSpan.FromSeconds(30)), "The threadpool callback did not run."); + Assert.True(callbackRan.Wait(TimeSpan.FromSeconds(30), TestContext.Current.CancellationToken), "The threadpool callback did not run."); } finally { From 9c2c8de357fd5e08a039cf13d63a348bfcad2bdc Mon Sep 17 00:00:00 2001 From: Jevan Saks Date: Fri, 3 Jul 2026 23:39:57 -0700 Subject: [PATCH 5/5] Remove flaky MiniDump self-dump test; cover null path via threadpool MiniDumpWriteDump on the current process suspends all other threads while it walks the process, so it can nondeterministically deadlock when a suspended thread holds a lock the dump needs. This reproduced as ~4/8 net10 test-host hangs locally (and a hang dump in CI) even with no managed callback involved. The #1739 regression (null optional non-blittable struct marshaling) is now covered entirely by ThreadpoolCallbackTests, which exercises both the non-null (single-element array) and null (null array -> null pointer) paths against the native threadpool without self-dumping. Stress-verified 12/12 clean on net10. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- test/GenerationSandbox.Tests/MiniDumpTests.cs | 46 ------------------- .../GenerationSandbox.Tests/NativeMethods.txt | 1 - .../ThreadpoolCallbackTests.cs | 32 +++++++++---- 3 files changed, 22 insertions(+), 57 deletions(-) delete mode 100644 test/GenerationSandbox.Tests/MiniDumpTests.cs diff --git a/test/GenerationSandbox.Tests/MiniDumpTests.cs b/test/GenerationSandbox.Tests/MiniDumpTests.cs deleted file mode 100644 index 03972757..00000000 --- a/test/GenerationSandbox.Tests/MiniDumpTests.cs +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.Diagnostics; -using System.Runtime.InteropServices; -using Windows.Win32; -using Windows.Win32.Foundation; -using Windows.Win32.System.Diagnostics.Debug; - -[Trait("WindowsOnly", "true")] -public class MiniDumpTests -{ - [Fact] - public void NullOptionalPointerParametersDoNotThrow() - { - // Regression test for https://github.com/microsoft/CsWin32/issues/1739. - // CallbackParam points at MINIDUMP_CALLBACK_INFORMATION, which is non-blittable - // (it contains a delegate field). Passing null for it must marshal to a null - // pointer rather than causing the marshaler to dereference a null reference. - // - // Note: this deliberately passes null for CallbackParam so that no managed callback - // runs while the process is being dumped. Supplying a callback here would let dbghelp - // invoke managed code while other threads are suspended for the dump, which can - // deadlock; the non-null marshaling is instead verified at runtime by - // ThreadpoolCallbackTests.NonNullOptionalNonBlittableStructIsMarshaled. - using Process process = Process.GetCurrentProcess(); - string dumpPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName()); - try - { - using FileStream dumpStream = File.Create(dumpPath); - BOOL result = PInvoke.MiniDumpWriteDump( - process.SafeHandle, - (uint)process.Id, - dumpStream.SafeFileHandle, - MINIDUMP_TYPE.MiniDumpNormal, - ExceptionParam: null, - UserStreamParam: null, - CallbackParam: null); - Assert.True(result, $"MiniDumpWriteDump failed with error 0x{Marshal.GetLastWin32Error():X}."); - } - finally - { - File.Delete(dumpPath); - } - } -} diff --git a/test/GenerationSandbox.Tests/NativeMethods.txt b/test/GenerationSandbox.Tests/NativeMethods.txt index 19a8ee6c..d0e25726 100644 --- a/test/GenerationSandbox.Tests/NativeMethods.txt +++ b/test/GenerationSandbox.Tests/NativeMethods.txt @@ -54,7 +54,6 @@ MAKELPARAM MAKELRESULT MAKEWPARAM MAX_PATH -MiniDumpWriteDump NTSTATUS PAGESET PathParseIconLocation diff --git a/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs b/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs index a19bd5e9..d163828d 100644 --- a/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs +++ b/test/GenerationSandbox.Tests/ThreadpoolCallbackTests.cs @@ -9,23 +9,35 @@ [Trait("WindowsOnly", "true")] public class ThreadpoolCallbackTests { + // Regression tests for https://github.com/microsoft/CsWin32/issues/1739. + // TP_CALLBACK_ENVIRON_V3 is non-blittable (it contains delegate fields), so in the default + // marshaling mode CsWin32 exposes the optional pcbe parameter as a nullable value type and + // forwards it to the native method through an array: a non-null value becomes a single-element + // array and null becomes a null array. These tests exercise both paths end to end by actually + // submitting work to the native threadpool and confirming the callback runs. Passing null must + // marshal to a null pointer rather than dereferencing a null reference (the original bug). [Fact] - public unsafe void NonNullOptionalNonBlittableStructIsMarshaled() + public void NonNullOptionalNonBlittableStructIsMarshaled() { - // Regression test for https://github.com/microsoft/CsWin32/issues/1739. - // TP_CALLBACK_ENVIRON_V3 is non-blittable (it contains delegate fields), so in the default - // marshaling mode CsWin32 exposes the optional pcbe parameter as a nullable value type and - // forwards a non-null value through a single-element array. This test exercises that array - // marshaling path end to end: the environment must reach the native threadpool intact for - // the submitted callback to actually run. - using ManualResetEventSlim callbackRan = new(false); - PTP_SIMPLE_CALLBACK callback = (instance, context) => callbackRan.Set(); - TP_CALLBACK_ENVIRON_V3 environment = default; environment.Version = 3; environment.CallbackPriority = TP_CALLBACK_PRIORITY.TP_CALLBACK_PRIORITY_NORMAL; environment.Size = (uint)Marshal.SizeOf(); + AssertCallbackRuns(environment); + } + + [Fact] + public void NullOptionalNonBlittableStructDoesNotThrow() + { + AssertCallbackRuns(null); + } + + private static unsafe void AssertCallbackRuns(TP_CALLBACK_ENVIRON_V3? environment) + { + using ManualResetEventSlim callbackRan = new(false); + PTP_SIMPLE_CALLBACK callback = (instance, context) => callbackRan.Set(); + try { BOOL submitted = PInvoke.TrySubmitThreadpoolCallback(callback, environment);