From a9ff969d98a62c69d43c66f4fd4c4e7104040816 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 22:10:15 +0000 Subject: [PATCH 1/4] Initial plan From 0f7be54d50a64b4e853a0a1fa99f6a73eb59cac5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 22:42:47 +0000 Subject: [PATCH 2/4] Fix marshalling generators using 0 as length during cleanup for inner arrays of jagged 2D arrays When the marshalling direction is ManagedToUnmanaged and there is no explicit count info (NoCountInfo), the cleanup code was setting numElements = 0, causing Free to never be called on inner array elements. This fix uses GetManagedValuesSource().Length instead to get the actual element count from the managed collection. Fixes #93423 Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/3665c6a3-655d-4215-a791-d3a838f80bd4 Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com> --- .../Marshalling/ElementsMarshalling.cs | 5 +++++ .../StatelessMarshallingStrategy.cs | 12 +++--------- .../Compiles.cs | 2 ++ .../CustomCollectionMarshallingCodeSnippets.cs | 18 ++++++++++++++++++ .../Compiles.cs | 2 ++ 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs index f21125e297aa7e..ed58e38db339a1 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs @@ -105,6 +105,11 @@ ExpressionSyntax GetExpressionForParam(TypePositionInfo paramInfo) public abstract StatementSyntax GenerateUnmarshalStatement(StubIdentifierContext context); public abstract StatementSyntax GenerateElementCleanupStatement(StubIdentifierContext context); + + public StatementSyntax GenerateNumElementsAssignmentFromManagedValuesSource(StubIdentifierContext context) + { + return CollectionSource.GetNumElementsAssignmentFromManagedValuesSource(CollectionSource.TypeInfo, context); + } } file static class ElementsMarshallingCollectionSourceExtensions diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/StatelessMarshallingStrategy.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/StatelessMarshallingStrategy.cs index da13f490614238..31363037d8f2b1 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/StatelessMarshallingStrategy.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/StatelessMarshallingStrategy.cs @@ -581,15 +581,9 @@ public IEnumerable GenerateCleanupCallerAllocatedResourcesState if (countInfo is NoCountInfo && MarshallerHelpers.GetMarshalDirection(TypeInfo, CodeContext) == MarshalDirection.ManagedToUnmanaged) { // When marshalling from managed to unmanaged, we may not have count info. - // For now, just set to 0. - // See https://github.com/dotnet/runtime/issues/93423 for a tracking issue. - - // = 0; - yield return ExpressionStatement( - AssignmentExpression( - SyntaxKind.SimpleAssignmentExpression, - IdentifierName(numElementsIdentifier), - LiteralExpression(SyntaxKind.NumericLiteralExpression, Literal(0)))); + // Use the managed collection's length to determine the number of elements to clean up. + // = .Length; + yield return elementsMarshalling.GenerateNumElementsAssignmentFromManagedValuesSource(context); } else { diff --git a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/Compiles.cs b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/Compiles.cs index c0806b2853d8f5..e7b81aa7c1d7cb 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/Compiles.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/ComInterfaceGenerator.Unit.Tests/Compiles.cs @@ -314,11 +314,13 @@ public static IEnumerable CustomCollectionsManagedToUnmanaged(Generato yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateless.NativeToManagedOnlyOutParameter() }; yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateless.NativeToManagedOnlyReturnValue() }; yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateless.NonBlittableElementByValue }; + yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateless.NonBlittableElementWithFreeByValue }; yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateless.NonBlittableElementNativeToManagedOnlyOutParameter }; yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateless.NonBlittableElementNativeToManagedOnlyReturnValue }; yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateful.NativeToManagedOnlyOutParameter() }; yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateful.NativeToManagedOnlyReturnValue() }; yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateful.NonBlittableElementByValue }; + yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateful.NonBlittableElementWithFreeByValue }; yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateful.NonBlittableElementNativeToManagedOnlyOutParameter }; yield return new[] { ID(), customCollectionMarshallingCodeSnippetsManagedToUnmanaged.Stateful.NonBlittableElementNativeToManagedOnlyReturnValue }; } diff --git a/src/libraries/System.Runtime.InteropServices/tests/Common/CustomCollectionMarshallingCodeSnippets.cs b/src/libraries/System.Runtime.InteropServices/tests/Common/CustomCollectionMarshallingCodeSnippets.cs index f25a50341e9d00..27a83c0aa5a15b 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/Common/CustomCollectionMarshallingCodeSnippets.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/Common/CustomCollectionMarshallingCodeSnippets.cs @@ -54,6 +54,16 @@ public struct Native { } public static Element ConvertToManaged(Native n) => throw null; } """; + public const string ElementInWithFree = """ + [CustomMarshaller(typeof(Element), MarshalMode.ElementIn, typeof(ElementMarshaller))] + static class ElementMarshaller + { + public struct Native { } + public static Native ConvertToUnmanaged(Element e) => throw null; + public static Element ConvertToManaged(Native n) => throw null; + public static void Free(Native unmanaged) { } + } + """; public const string ElementOut = """ [CustomMarshaller(typeof(Element), MarshalMode.ElementOut, typeof(ElementMarshaller))] static class ElementMarshaller @@ -252,6 +262,10 @@ public string NestedMarshallerParametersAndModifiers(string elementType) => _pro + NonBlittableElement + ElementIn; + public string NonBlittableElementWithFreeByValue => ByValue("Element") + + NonBlittableElement + + ElementInWithFree; + public string NonBlittableElementNativeToManagedOnlyOutParameter => NativeToManagedOnlyOutParameter("Element") + NonBlittableElement + ElementOut; @@ -508,6 +522,10 @@ public string NativeToManagedFinallyOnlyReturnValue(string elementType) => _snip + NonBlittableElement + ElementIn; + public string NonBlittableElementWithFreeByValue => ByValue("Element") + + NonBlittableElement + + ElementInWithFree; + public string NonBlittableElementNativeToManagedOnlyOutParameter => NativeToManagedOnlyOutParameter("Element") + NonBlittableElement + ElementOut; diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs index 81b10efc82d4af..d11f2942f4ec9f 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs @@ -394,6 +394,7 @@ public static IEnumerable CustomCollections() yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateless.NativeToManagedFinallyOnlyReturnValue() }; yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateless.NestedMarshallerParametersAndModifiers() }; yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateless.NonBlittableElementByValue }; + yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateless.NonBlittableElementWithFreeByValue }; yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateless.NonBlittableElementParametersAndModifiers }; yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateless.NonBlittableElementNativeToManagedOnlyOutParameter }; yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateless.NonBlittableElementNativeToManagedFinallyOnlyOutParameter }; @@ -432,6 +433,7 @@ public static IEnumerable CustomCollections() yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateful.NativeToManagedOnlyReturnValue() }; yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateful.NativeToManagedFinallyOnlyReturnValue() }; yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateful.NonBlittableElementByValue }; + yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateful.NonBlittableElementWithFreeByValue }; yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateful.NonBlittableElementParametersAndModifiers }; yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateful.NonBlittableElementNativeToManagedOnlyOutParameter }; yield return new[] { ID(), customCollectionMarshallingCodeSnippets.Stateful.NonBlittableElementNativeToManagedFinallyOnlyOutParameter }; From 6b15de0d3d3af6dc9e9a57b823eae5582869f244 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 22:45:16 +0000 Subject: [PATCH 3/4] Add XML documentation to GenerateNumElementsAssignmentFromManagedValuesSource Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/3665c6a3-655d-4215-a791-d3a838f80bd4 Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com> --- .../Marshalling/ElementsMarshalling.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs index ed58e38db339a1..16fbe1d2393b71 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs @@ -106,6 +106,11 @@ ExpressionSyntax GetExpressionForParam(TypePositionInfo paramInfo) public abstract StatementSyntax GenerateUnmarshalStatement(StubIdentifierContext context); public abstract StatementSyntax GenerateElementCleanupStatement(StubIdentifierContext context); + /// + /// + /// <numElements> = <GetManagedValuesSource>.Length; + /// + /// public StatementSyntax GenerateNumElementsAssignmentFromManagedValuesSource(StubIdentifierContext context) { return CollectionSource.GetNumElementsAssignmentFromManagedValuesSource(CollectionSource.TypeInfo, context); From 3ff90d52fbe82b3057303bb24e3c72657d531d01 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:38:09 +0000 Subject: [PATCH 4/4] Fix nested cleanup codegen and enable behavioral tests for #93423 The previous fix only changed the inner numElements assignment but did not declare the outer managedSpan in the cleanup scope, causing a compile error in generated code for nested collections. This commit: - Declares managedSpan in NonBlittableElementsMarshalling.GenerateElementCleanupStatement when direction is ManagedToUnmanaged, so inner cleanup can reference it. - Removes [ActiveIssue("#93423")] from MultidimensionalArray_CheckInnerArraysAreCleared and updates expected free count to throwOn - throwOn % 10 (only fully completed inner arrays are cleaned up; the remaining gap is tracked by #93431). - Splits a separate MultidimensionalArray_CheckInnerArraysAreCleared_ProperCleanup test marked with [ActiveIssue(#93431)] for the partial inner cleanup case. - Uncomments the BoolStructInMarshaller asserts in MultiDimensionalOutArray_EnsureAllCleaned which now correctly free all 100 inner elements on success and failure paths. Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/1d08ed99-2083-4c06-a7c1-15d6a4159e8e Co-authored-by: jtschuster <36744439+jtschuster@users.noreply.github.com> --- .../Marshalling/ElementsMarshalling.cs | 32 +++++++++++++--- .../CollectionMarshallingFails.cs | 37 +++++++++++++------ 2 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs index 16fbe1d2393b71..c547efe7e46d53 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/Marshalling/ElementsMarshalling.cs @@ -376,6 +376,7 @@ public override StatementSyntax GenerateManagedToUnmanagedByValueOutUnmarshalSta public override StatementSyntax GenerateElementCleanupStatement(StubIdentifierContext context) { string nativeSpanIdentifier = MarshallerHelpers.GetNativeSpanIdentifier(CollectionSource.TypeInfo, context); + string managedSpanIdentifier = MarshallerHelpers.GetManagedSpanIdentifier(CollectionSource.TypeInfo, context); ExpressionSyntax indexConstraintName; if (!UsesLastIndexMarshalled(CollectionSource.TypeInfo, CollectionSource.CodeContext)) { @@ -404,13 +405,32 @@ public override StatementSyntax GenerateElementCleanupStatement(StubIdentifierCo return EmptyStatement(); } + // Declare the managed span so that nested element cleanup can access it via [] + // (e.g., when an inner stateless collection with NoCountInfo needs to compute its length from the + // managed source during ManagedToUnmanaged cleanup). + bool isManagedToUnmanaged = MarshallerHelpers.GetMarshalDirection(CollectionSource.TypeInfo, CollectionSource.CodeContext) == MarshalDirection.ManagedToUnmanaged; + LocalDeclarationStatementSyntax nativeSpanDeclaration = Declare( + ReadOnlySpanOf(unmanagedElementType), + nativeSpanIdentifier, + isManagedToUnmanaged + ? CollectionSource.GetUnmanagedValuesDestination(context) + : CollectionSource.GetUnmanagedValuesSource(context)); + + if (isManagedToUnmanaged) + { + LocalDeclarationStatementSyntax managedSpanDeclaration = Declare( + ReadOnlySpanOf(elementMarshaller.TypeInfo.ManagedType.Syntax), + managedSpanIdentifier, + CollectionSource.GetManagedValuesSource(context)); + + return Block( + nativeSpanDeclaration, + managedSpanDeclaration, + contentsCleanupStatements); + } + return Block( - Declare( - ReadOnlySpanOf(unmanagedElementType), - nativeSpanIdentifier, - MarshallerHelpers.GetMarshalDirection(CollectionSource.TypeInfo, CollectionSource.CodeContext) == MarshalDirection.ManagedToUnmanaged - ? CollectionSource.GetUnmanagedValuesDestination(context) - : CollectionSource.GetUnmanagedValuesSource(context)), + nativeSpanDeclaration, contentsCleanupStatements); } diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionMarshallingFails.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionMarshallingFails.cs index dd2759297b3a20..09dbefdea4f783 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionMarshallingFails.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.Tests/CollectionMarshallingFails.cs @@ -177,13 +177,33 @@ public void MultidimensionalArray_CheckOuterArrayIsIndexTracked() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/93423")] public void MultidimensionalArray_CheckInnerArraysAreCleared() { var arr = GetMultiDimensionalArray(10, 10); foreach (var throwOn in new int[] { 0, 1, 45, 99 }) { BoolStructInMarshallerAllowNull.Marshaller.MarshallingFailsIndex = throwOn; + // https://github.com/dotnet/runtime/issues/93431 + // We currently only clean up inner arrays that were fully marshalled + // (i.e. all elements in the outer collection up to the last fully completed inner array). + BoolStructInMarshallerAllowNull.Marshaller.ExpectedFreeCount = throwOn - throwOn % 10; + Assert.Throws(() => + { + NativeExportsNE.MarshallingFails.MarshalMultidimensionalArray_CheckInnerArraysAreCleared(arr); + }); + BoolStructInMarshallerAllowNull.Marshaller.AssertAllHaveBeenCleaned(); + } + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/93431")] + public void MultidimensionalArray_CheckInnerArraysAreCleared_ProperCleanup() + { + var arr = GetMultiDimensionalArray(10, 10); + foreach (var throwOn in new int[] { 0, 1, 45, 99 }) + { + BoolStructInMarshallerAllowNull.Marshaller.MarshallingFailsIndex = throwOn; + // Expected Behavior - Should free all elements of inner arrays that were partially marshalled BoolStructInMarshallerAllowNull.Marshaller.ExpectedFreeCount = throwOn; Assert.Throws(() => { @@ -230,27 +250,20 @@ public void MultiDimensionalOutArray_EnsureAllCleaned() // Set up unmarshalling asserts BoolStructOutMarshaller.Marshaller.UnmarshallingFailsIndex = throwOn; BoolStructOutMarshaller.Marshaller.ExpectedFreedValues = Enumerable.Range(0, 100).Select(_ => new BoolStructNative() { b1 = 1, b2 = 1, b3 = 1 }).ToArray(); - // https://github.com/dotnet/runtime/issues/93423 - //NegateBoolStructInMarshaller.Marshaller.ExpectedFreedValues = Enumerable.Range(0, 100).Select(_ => new BoolStructNative() { b1 = 0, b2 = 0, b3 = 0 }).ToArray(); + BoolStructInMarshaller.Marshaller.ExpectedFreedValues = Enumerable.Range(0, 100).Select(_ => new BoolStructNative() { b1 = 0, b2 = 0, b3 = 0 }).ToArray(); Assert.Throws(() => { NativeExportsNE.MarshallingFails.NegateBoolsOut2D(arr, arr.Length, widths, out BoolStruct[][] boolsOut); }); - // https://github.com/dotnet/runtime/issues/93423 - //NegateBoolStructInMarshaller.Marshaller.AssertAllHaveBeenCleaned(); - BoolStructInMarshaller.Marshaller.Reset(); + BoolStructInMarshaller.Marshaller.AssertAllHaveBeenCleaned(); BoolStructOutMarshaller.Marshaller.AssertAllHaveBeenCleaned(); } // Run without throwing - this is okay only because the native code doesn't actually use the array, it creates a whole new one BoolStructOutMarshaller.Marshaller.UnmarshallingFailsIndex = -1; BoolStructOutMarshaller.Marshaller.ExpectedFreedValues = Enumerable.Range(0, 100).Select(_ => new BoolStructNative() { b1 = 1, b2 = 1, b3 = 1 }).ToArray(); - // https://github.com/dotnet/runtime/issues/93423 - //NegateBoolStructInMarshaller.Marshaller.UnmarshallingFailsIndex = -1; - //NegateBoolStructInMarshaller.Marshaller.ExpectedFreeCount = 100; + BoolStructInMarshaller.Marshaller.ExpectedFreedValues = Enumerable.Range(0, 100).Select(_ => new BoolStructNative() { b1 = 0, b2 = 0, b3 = 0 }).ToArray(); NativeExportsNE.MarshallingFails.NegateBoolsOut2D(arr, arr.Length, widths, out BoolStruct[][] boolsOut); - // https://github.com/dotnet/runtime/issues/93423 - //NegateBoolStructInMarshaller.Marshaller.AssertAllHaveBeenCleaned(); - BoolStructInMarshaller.Marshaller.Reset(); + BoolStructInMarshaller.Marshaller.AssertAllHaveBeenCleaned(); BoolStructOutMarshaller.Marshaller.AssertAllHaveBeenCleaned(); }