Fix SafeHandle debug-only _ctorStackTrace for mono

[stephentoub]

Try to fix mono failure
Fixes #82308

[stephentoub]

/azp list

[azure-pipelines]

CI/CD Pipelines for this repository:

• runtime-coreclr outerloop
• runtime-coreclr jitstress
• runtime-coreclr jitstressregs
• runtime-coreclr jitstress2-jitstressregs
• runtime-coreclr gcstress0x3-gcstress0xc
• runtime-coreclr gcstress-extra
• runtime-coreclr r2r-extra
• runtime-coreclr jitstress-isas-x86
• runtime-coreclr jitstress-isas-arm
• runtime-coreclr jitstressregs-x86
• runtime-coreclr libraries-jitstressregs
• runtime-coreclr libraries-jitstress2-jitstressregs
• runtime-coreclr r2r
• runtime-coreclr runincontext
• runtime-coreclr crossgen2
• runtime-libraries-coreclr outerloop
• runtime-libraries-coreclr outerloop-windows
• runtime-libraries-coreclr outerloop-linux
• runtime-libraries-coreclr outerloop-osx
• runtime
• runtime-libraries enterprise-linux
• runtime-libraries stress-http
• runtime-libraries stress-ssl
• runtime-dev-innerloop
• runtime-coreclr crossgen2 outerloop
• coreclr-release-outerloop-nightly
• runtime-coreclr crossgen2-composite
• runtime-jit-experimental
• runtime-coreclr libraries-jitstress
• dotnet-linker-tests
• runtime-coreclr ilasm
• runtime-coreclr crossgen2-composite gcstress
• runtime-staging
• runtime-coreclr pgo
• runtime-coreclr libraries-pgo
• Antigen
• runtime-community
• Fuzzlyn
• runtime-coreclr superpmi-replay
• runtime-wasm
• runtime-coreclr superpmi-diffs
• runtime-coreclr superpmi-asmdiffs-checked-release
• runtime-extra-platforms
• jit-cfg
• runtime-wasm-perf
• runtime-llvm
• runtime-coreclr jitstress-random
• runtime-coreclr libraries-jitstress-random
• runtime-android-grpc-client-tests
• runtime-wasm-libtests
• runtime-wasm-non-libtests
• runtime-android
• runtime-androidemulator
• runtime-ioslike
• runtime-ioslikesimulator
• runtime-linuxbionic
• runtime-maccatalyst
• runtime-coreclr pgostress
• runtime-coreclr jitstress-isas-avx512
• runtime-tools-tests
• runtime-libraries-mono outerloop
• runtime-sanitized
• runtime-wasm-dbgtests
• runtime-wasm-optional

[stephentoub]

/azp run runtime-community

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[stephentoub]

/azp run runtime-extra-platforms

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[lambdageek]

This is technically fine, but maybe we should do something a bit cleaner.

Also I really think we need

#if MONO
[StructLayout(LayoutKind.Sequential)]
#endif

[lambdageek]

@stephentoub questions:

1. Can we move this member to the end of the set of instance members. (And I assume do the related changes on the native side in coreclr)
2. For Mono we should really have [StructLayout(Sequential)] on any class where we care about the field layout. Otherwise we're just relying on auto layout coincidentally getting the fields in the right order.
3. Should we make SafeHandle.Mono.cs and SafeHandle.CoreCLR.cs that just contain the instance field definitions? #if DEBUG && CORECLR accomplishes the same thing but feels more fragile.

[stephentoub]

Can we move this member to the end of the set of instance members. (And I assume do the related changes on the native side in coreclr)

I'd previously done that, and I believe the issues I hit were to do with alignment, though it's also possible I just messed something up or neglected to edit something I needed to edit.

For Mono we should really have [StructLayout(Sequential)] on any class where we care about the field layout. Otherwise we're just relying on auto layout coincidentally getting the fields in the right order.

This is irrespective of this change or this field, right? You're saying mono is expecting that handle is first and it may not be.

Should we make SafeHandle.Mono.cs and SafeHandle.CoreCLR.cs that just contain the instance field definitions? #if DEBUG && CORECLR accomplishes the same thing but feels more fragile.

I don't love the duplication there; that feels more fragile to me, actually. If this field is problematic, we can just remove it and I can put it back in the future should someone want to do another push like I did to drive down SafeHandle finalization debt.

[lambdageek]

This is irrespective of this change or this field, right? You're saying mono is expecting that handle is first and it may not be.

Yea, it's not required for this PR (all the other fields are non-reference so auto layout will get things right). Just might as well do some future-proofing

[lambdageek]

Overall I'm fine with the PR as is. It would be nice to get the layout attribute in here, but we could also do a separate PR for that. If splitting the file feels more fragile to you @stephentoub , I'm ok with the PR as is.

[stephentoub]

@lambdageek, mono actually already has StructLayout applied:

runtime/src/mono/System.Private.CoreLib/src/System/Runtime/InteropServices/SafeHandle.Mono.cs

Lines 6 to 8 in 5e3dd8d

	// Mono runtime relies on exact layout
	[StructLayout(LayoutKind.Sequential)]
	public abstract partial class SafeHandle