Tagging subscribers to this area:@safern,@tarekgh
See info inarea-owners.md if you want to be subscribed.

@ericstj@ViktorHofer - this is another case where we are splitting ".NET Core 3.1" build with ".NET Current" build. (I had to do that in order to use ComWrappers, because that API came in 5.0.) This means we are going to be missing test coverage for all of these COM interop scenarios for System.Drawing.Common v6.0.0 when running on .NET Core 3.1.

I agree that is painful and should be fixed by adding support for testing on older .NETCoreApp versions and run them either per rolling build or nightly. We don't have capacity to run them on every PR.

@ViktorHofer what if this was opt-in per test? A test project could decide to add a configuration fornetcore3.1-Windows and we could have one of the build legs (either windows or all-configurations) create the helix work items. Would this require more capacity than a single test additional test assembly?

cc@michaelgsharp as well, who's been doing a lot of helix work lately. ML.NET has a similar requirement around wanting to run tests against multiple different framework versions.

Also, maybe we should move this discussion to an issue. Do we have a good one already?

@ViktorHofer what if this was opt-in per test? A test project could decide to add a configuration for netcore3.1-Windows and we could have one of the build legs (either windows or all-configurations) create the helix work items. Would this require more capacity than a single test additional test assembly?

The overall issue isn't specific to Windows, we need coverage on as many OSs that a library supports, as possible. The cost that I'm concerned about isn't observable in the amount of build agents but in the number of Helix clients that accept work items.

I imagine support for testing on older .NETCoreApp configurations could be implemented in the following way in order:

• Downgrade the TestUtilities.csproj to netcoreapp3.1 (use$(NetCoreAppMinimum) which will be introduced soon).
• Add infra support for running .NETCoreApp tests without a testhost (same as for .NETFramework testing)here.
• Add two legs (or as an optimization only one leg) for netcoreapp3.1 and net5.0 that build the respective configuration and submit jobs to helix for all OSs. Consider adding a new pipeline for that so that it can be scheduled (i.e. nightly).
• Add netcoreapp3.1/net5.0 TFMs in the OOB test projects either everywhere or on a per project basis where it makes sense (I think the former would make more sense).
• Make sure that the new pipeline is monitored to catch regressions in the older .NETCoreApp TFM assets.

Moved test infrastructure discussion into#54639.

Looks like the x86 tests are failing:

Fatal error. 0xC0000005   at System.Runtime.InteropServices.Marshal.Release(IntPtr)   at System.Drawing.Icon.Save(System.IO.Stream)   at System.Drawing.Tests.IconTests.SaveAndCompare(System.Drawing.Icon, Boolean)   at System.Drawing.Tests.IconTests.FromHandle_IconHandleMultipleTime_Success()

Trying to get a repro locally.

@ViktorHofer do we have a lot of projects that would run on previous versions of .NET Core? What if we start small and run them on Ubuntu, Windows and macOS on the build agent without using helix? If there aren't may projects that would want to do this, we could get away without using helix and running them on the build agent.

Looks like the x86 tests are failing:

This is what I see when trying to debug the .dmp file.@AaronRobinsonMSFT any thoughts?

Thread  11Current frame: coreclr!GetCLRRuntimeHost + 0x43f03ChildEBP RetAddr  Caller, Callee072AD388 72d194e2 coreclr!coreclr_shutdown_2 + 0x94a52, calling coreclr!coreclr_shutdown_2 + 0x109cc072AD470 72d194e2 coreclr!coreclr_shutdown_2 + 0x94a52, calling coreclr!coreclr_shutdown_2 + 0x109cc072AD484 72dabc83 coreclr!GetCLRRuntimeHost + 0x43f03, calling coreclr!coreclr_shutdown_2 + 0x2f670072AD698 72c3b2d4 coreclr!coreclr_initialize + 0x42f4, calling coreclr!coreclr_initialize + 0x430b072AD6A8 72d194ef coreclr!coreclr_shutdown_2 + 0x94a5f, calling coreclr!GetCLRRuntimeHost + 0x43e91072AD78C 72c8dec3 coreclr!coreclr_shutdown_2 + 0x9433, calling coreclr!coreclr_shutdown_2 + 0x945f072AD80C 72be1501 coreclr + 0x61501, calling coreclr!coreclr_shutdown_2 + 0x169a3072AD810 72be1462 coreclr + 0x61462, calling coreclr!coreclr_shutdown_2 + 0x169a3072AD828 72be04fe coreclr + 0x604fe, calling coreclr + 0x60517072AD86C 771980a9 ntdll!ExecuteHandler2 + 0x26072AD890 7719807b ntdll!ExecuteHandler + 0x24, calling ntdll!ExecuteHandler2072AD8B4 7719801d ntdll!RtlDispatchException + 0x127, calling ntdll!RtlpExecuteHandlerForException072AD940 77150133 ntdll!KiUserExceptionDispatcher + 0xf, calling ntdll!RtlDispatchException072ADE0C 90ffffd4 90ffffd4 ====> Exception cxr@072AD9A8072ADB94 77172d9a ntdll!RtlpFreeHeap + 0x1f4, calling ntdll!RtlpCoalesceFreeBlocks072ADBE4 0735c49a (MethodDesc 0038f78c + 0xa System.Buffer.Memmove[[System.Byte, System.Private.CoreLib]](Byte ByRef, Byte ByRef, UIntPtr)), calling (MethodDesc 00270f28 + 0 System.Buffer.Memmove(Byte ByRef, Byte ByRef, UIntPtr))072ADD08 72bd86bc coreclr + 0x586bc, calling coreclr!coreclr_shutdown_2 + 0x169a3072ADD0C 72bd80bd coreclr + 0x580bd, calling coreclr + 0x58149072ADD1C 72bd810d coreclr + 0x5810d, calling coreclr!coreclr_shutdown_2 + 0x169a3072ADD74 76615847 gdi32!IcmReleaseCachedColorSpace + 0x41, calling ntdll!RtlLeaveCriticalSection072ADD88 7716e143 ntdll!RtlFreeHeap + 0x105, calling ntdll!RtlpLowFragHeapFree072ADDA0 76c56eaa ole32!CRetailMalloc_Free + 0x1c [d:\w7rtm\com\ole32\com\class\memapi.cxx:687], calling ntdll!RtlFreeHeap072ADDB4 748b470c oleaut32!MemFree + 0x4e072ADDC0 748d357c oleaut32!CPicture::~CPicture + 0xd7, calling oleaut32!_EH_epilog3072ADDC8 748b4795 oleaut32!operator delete + 0xd, calling oleaut32!MemFree072ADDD4 748d34ba oleaut32!CPicture::`scalar deleting destructor' + 0x19, calling oleaut32!operator delete072ADDE4 748d349a oleaut32!CPicture::Release + 0x27, calling oleaut32!CPicture::`scalar deleting destructor'072ADE08 0881ce31 (MethodDesc 002e28f4 + 0x99 System.Runtime.InteropServices.Marshal.Release(IntPtr))072ADE24 0881ce31 (MethodDesc 002e28f4 + 0x99 System.Runtime.InteropServices.Marshal.Release(IntPtr))072ADE58 08c5b0da (MethodDesc 02388b30 + 0x2b2 System.Drawing.Icon.Save(System.IO.Stream)), calling (MethodDesc 002e28f4 + 0 System.Runtime.InteropServices.Marshal.Release(IntPtr))072ADF08 0881b5c2 (MethodDesc 0252e85c + 0x82 System.Drawing.Tests.IconTests.SaveAndCompare(System.Drawing.Icon, Boolean)), calling (MethodDesc 02388b30 + 0 System.Drawing.Icon.Save(System.IO.Stream))

@eerhardt How can I get a hold of that DMP file?

I have it locally, and can send it to you. Or:

dotnet tool install --global runfodotnet tool update --global runforunfo get-helix-payload -j 2ebc65eb-6738-4487-9823-d41180299501 -w System.Drawing.Common.Tests -o %WOUTDIR%

Seehttps://github.com/dotnet/runtime/blob/main/eng/testing/debug-dump-template.md

Lineusing DrawingComWrappers.IPicture picture = (DrawingComWrappers.IPicture)DrawingComWrappers.Instance.GetOrCreateObjectForComInstance(lpPicture, CreateObjectFlags.None); already release instance of lpPicture in Dispose.

https://github.com/dotnet/runtime/pull/54636/files#diff-f587d46981bfe7aeb74b6390ab4035ef7355e1c5cfe849583efb14da68c0aa49R288 maybe here should be added AddRef.

Line using DrawingComWrappers.IPicture picture = (DrawingComWrappers.IPicture)DrawingComWrappers.Instance.GetOrCreateObjectForComInstance(lpPicture, CreateObjectFlags.None); already release instance of lpPicture in Dispose.

The AddRef happens (in QueryInterface) before the PictureWrapper class is created, here:

https://github.com/dotnet/runtime/pull/54636/files#diff-f587d46981bfe7aeb74b6390ab4035ef7355e1c5cfe849583efb14da68c0aa49R54-R58

Notice that x64 passes. It is just failing on x86.

There are also x64 tests failing, and these I can repro on my machine, but not 100% of the time.

The test failure is:

Assert.Throws() FailureExpected: typeof(System.ObjectDisposedException)Actual:   typeof(System.NotSupportedException): Specified method is not supported.---- System.NotSupportedException : Specified method is not supported.

We are expecting anObjectDisposedException, but aNotSupportedException is being thrown. This appears to be a quirk/bug withMarshal.ThrowExceptionForHR. I got it to repro on my machine and here's what I see in the debugger:

0x80131622 is the error code forObjectDisposedException:

ButMarshal.ThrowExceptionForHR is throwingNotSupportedException, which is odd.@jkotas (or anyone else) have you ever seen this before? The only thing I found while searching was#47329, but that seems like something else.

There are also x64 tests failing

I was able to catch some of these in a debugger, and have figured out some more information. The case where it is failing is getting into:

pErrInfo is non-null. When this occurs, the code inside thatif is settingpProtectedThrowable to aNotSupportedException, and that is what is getting returned to managed code and getting thrown byMarshal.ThrowExceptionForHR, even though I explicitly passed in0x80131622 (COR_E_OBJECTDISPOSED) to the method.

AFAICT, the reasonpErrInfo is non-null is because of this line in the calling method:

The managed C# code is passing inNULL for the error info, but this line is reading it fromSafeGetErrorInfo. Sometimes that is returningS_OK and an error info. From where? I don't have a clue 😄. But it feels like someone isn't clearing the previous error info here.

A naïve question: If I'm callingMarshal.ThrowExceptionForHR(int) and not passing in anerrorInfo, should this code even be readingSafeGetErrorInfo? It looks like there is support to pass-1 to the native code, and it won't read it. So should this C# line

be changed toGetExceptionForHR(errorCode, new IntPtr(-1));?

Yes, COM IErrorInfo is mess. .NET interop assumes that all COM calls set it, but it is not the case in practice.

If I'm calling Marshal.ThrowExceptionForHR(int) and not passing in an errorInfo, should this code even be reading SafeGetErrorInfo?

Changing that would be breaking behavior change. It should be fine to pass innew IntPtr(-1) in the wrappers you have written in this PR.

The musl library test leg is failing HTTP and sockets tests - unrelated to this change.

The CoreClr Pri0 arm64 test failure also look unrelated, but I can’t find a current issue tracking the failures.

Test failures are unrelated. Merging.