-
Notifications
You must be signed in to change notification settings - Fork 5k
Distributed transactions: AccessViolationException on arm64 #74170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue Detailswin-arm64 libraries random JitStress failure
@dotnet/jit-contrib
|
Could this be related to recent changes in this area? I am able to repro this even without the jitstress flags. This was recently ported to Windows in #72051 and seems related to that. Here is the call stack on windows/arm64
|
Will look at this ASAP |
I would be curious to know why we don't see these failures in other CI pipelines. |
@roji I may have missed the by-ref for the |
@AaronRobinsonMSFT thanks, I'll prepare a PR for that tomorrow. Though the debugging session above seems to show that ppvResMgr is the source of the crash, which seems very odd (it's just an out parameter on the .NET side). And yeah, like @kunalspathak I'm wondering why this hasn't shown up before in CI... |
I can merge #74226, but @kunalspathak indicated above that the failure still occurs. Looking at the error again, this occurs here, one of the 1st interop calls done; ConnectToProxy is first thing that ever does interop in Sys.Tx. The stack trace in #74170 seems to indicate that the last parameter, ppvResMgr, points to an invalid address; since this is a plain .NET out variable, this seems like it could only happen if there's some issue with the signature of the CreateEx method (signature in this PR, docs), or something more low level. @AaronRobinsonMSFT note that there's interface inheritance going on here (IResourceManagerFactory.Create and IResourceManagerFactory2.CreateEx), in case that's related. With this happening only on arm64, and even there not deterministically, I'm out of my depth here... I've pushed a change here to disable the tests on ARM to unblock the build, and will merge. Hopefully we can resolve the issue soon and reenable. |
And disable tests on ARM for now. Works around #74170
NonMsdtcPromoterTests.PSPENonMsdtcGetPromoterTypeMSDTC was triggering an MSDTC distributed transaction on Windows, but without the proper checks/resiliency. Moved to OleTxTests. Fixes #74170
NonMsdtcPromoterTests.PSPENonMsdtcGetPromoterTypeMSDTC was triggering an MSDTC distributed transaction on Windows, but without the proper checks/resiliency. Moved to OleTxTests. Fixes #74170
NonMsdtcPromoterTests.PSPENonMsdtcGetPromoterTypeMSDTC was triggering an MSDTC distributed transaction on Windows, but without the proper checks/resiliency. Moved to OleTxTests. Fixes #74170 Co-authored-by: Shay Rojansky <[email protected]>
@kunalspathak @AaronRobinsonMSFT how can we make progress on this arm64-specific issue? @kunalspathak neither @AaronRobinsonMSFT nor I have access to an arm64 machine - are you able to help us iterate on this as per #74170 (comment)? I'm worried not so much about arm64 distributed transaction support, but more that there's some deeper bug in interop there, since the interop signatures seem to be fine - so we may want to prioritize this as a general arm64 bug. Is there someone we should loop in on this? If we think we can't manage to investigate this in time, I'll go ahead and a runtime check aganist arm64, so that users get a clean PlatformNotSupportedException right away rather than a non-deterministic AccessViolationException. |
I'm marking blocking-release so this stays on the radar. |
According to [the docs](https://docs.microsoft.com/en-us/previous-versions/windows/desktop/ms681318(v=vs.85)) and the Windows SDK headers, the Guid parameters here are all passed by-ref. Update the definition of the interface to pass the `Guid` parameters with `in` to match the native signature. Fixes dotnet#74170
And fix GUID interop in distributed transactions See dotnet#74170
See update in #74570 (comment) - I propose to block ARM for rc2 and am updating PRs #74226 and #74570 to do so. We can continue investigating in parallel, of course. |
And fix GUID interop in distributed transactions See #74170
@roji should we re-enable the tests and remove links to this issue? https://github.com/dotnet/runtime/search?q=74170 |
@karelz no, unfortunately the issue still persists... My proposal was to block distributed transactions on arm64 because of this, but we may want to allow it instead (see #74570 (comment)). In any case, the intention is to get to the bottom of this post-rc2, possibly post-7.0. |
And fix GUID interop in distributed transactions See #74170 (cherry picked from commit fdfef13) Co-authored-by: Shay Rojansky <[email protected]>
Due to a previous mis-communication, it was believed that we're still seeing AccessViolationException even after some interop fixes (see #74226). However, we're no longer able to reproduce the AccessViolationException. arm64 support has been re-enabled in #75703, including all testing. Closing this, will reopen if we see AccessViolationException again. |
Uh oh!
There was an error while loading. Please reload this page.
Happens a lot -- see last 30 days:
win-arm64 libraries random JitStress failure
@dotnet/jit-contrib
The text was updated successfully, but these errors were encountered: