Skip to content
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

[mono][threads] save/restore errno in thread state transitions #79856

Merged
merged 2 commits into from
Jul 12, 2023

Conversation

lambdageek
Copy link
Member

When we exit from GC Safe to GC Unsafe mode, if there is a STW in progress, the thread will self-suspend. On some platforms, self-suspend is implemented using POSIX semaphores sem_wait. Failures and spurious wakeups in sem_wait may change errno. That will make the following code (generated by the LibraryImportAttribute source generator) capture an incorrect error code:

__retVal = __PInvoke(__path_native, mode); // there is a pinvoke wrapper here, that transitions from GC Safe to GC Unsafe mode
__lastError = System.Runtime.InteropServices.Marshal.GetLastSystemError();

Note: we already had code to do this on win32 where APC calls can run on a suspended thread and change the value returned by GetLastError. This PR expands the code for other platforms.

Currently apple platforms use Mach semaphores ( semaphore_wait, etc) which do not pollute errno, so on those platforms we don't bother saving/restoring. If the thread state transition code changes to do additional work using other libc calls, we may need to revisit this.

Fixes #77364

We already save/restore GetLastError on win32.  Do it on posix
platforms, too.

If one thread is in a pinvoke wrapper, while another thread triggers a
STW, the pinvoke wrapper will self-suspend the thread and wait for a
notification to resume.  Depending on the platform we can use win32
primitives, Mach semaphores or POSIX semaphores.  win32 and posix can
both change the value of last error (errno, respectively) while the
thread is suspended.

That means that code like this (generated by the
LibraryImportAttribute source generator) cannot reliably retrieve the
error from the last pinvoke:

```csharp
__retVal = __PInvoke(__path_native, mode); // there is a pinvoke wrapper here, that transitions from GC Safe to GC Unsafe mode
__lastError = System.Runtime.InteropServices.Marshal.GetLastSystemError();
```

The solution is to explicitly preserve the value of GetLastError/errno
when exiting from GC Safe.

Fixes dotnet#77364
…RESTORE_POINT

and W32_RESTORE_LAST_ERROR_FROM_RESTORE_POINT to MONO_RESTORE_LAST_ERROR_FROM_RESTORE_POINT
@lambdageek
Copy link
Member Author

I'm not sure there's a reliable test case to be made here. I'm going to try to make something with a pinvoke to a C function that always sets errno together with a loop that repeatedly triggers a GC, but it would rarely catch the problem, I think.

@ayakael
Copy link
Contributor

ayakael commented Mar 1, 2023

I've tested this with dotnet8-preview1 on Alpine linux on ppc64le and s390x and this fixes the restore issue reliably. As for dotnet7, there's something missing for the bug to be fixed.

@lewing
Copy link
Member

lewing commented Jul 10, 2023

ping @lambdageek

@lewing lewing added this to the 8.0.0 milestone Jul 10, 2023
@lambdageek lambdageek closed this Jul 10, 2023
@lambdageek lambdageek reopened this Jul 10, 2023
@lambdageek
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants