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

Last p/invoke error is not 0 after calling a p/invoke with SetLastError=true that doesn't set the system error #51600

Closed
elinor-fung opened this issue Apr 20, 2021 · 2 comments · Fixed by #57127
Assignees
Milestone

Comments

@elinor-fung
Copy link
Member

elinor-fung commented Apr 20, 2021

See #51505 (comment).
Test case is under src/tests/Interop/PInvoke/SetLastError

For p/invokes with SetLastError=true, the system error should be set to 0 before calling the target function and the system error saved as the last p/invoke error after the call, such that if the target function doesn't update the system error, the stored p/invoke error will be 0. This does not seem to be the case on mono.

  • Call a P/Invoke that has SetLastError=true but that doesn't set the system error
  • Call Marshal.GetLastPInvokeError or Marshal.GetLastWin32Error

Example:

[DllImport("NativeLib", SetLastError = true)]
public static extern void SetError(int error, byte shouldSetError);
...
SetError(100, shouldSetError: 1);
// Marshal.GetLastPInvokeError/GetLastWin32Error return 100, as expected

SetError(50, shouldSetError: 0);
// Marshal.GetLastPInvokeError/GetLastWin32Error should return 0, but does not
extern "C" DLL_EXPORT void STDMETHODCALLTYPE SetError(int err, bool shouldSetError)
{
    if (!shouldSetError)
        return;

#ifdef WINDOWS
    ::SetLastError(err);
#else
    errno = err;
#endif
}
@lambdageek
Copy link
Member

#51505 (comment)

Should work the same way as CoreCLR - errno should be set to 0 before the call, and then after the call mono_marshal_set_last_error should get called to push the current errno value to the last pinvoke error thread-local variable.

It's possible that it isn't working right (maybe while JITing the wrapper method for the pinvoke, the JIT sets errno to 1 for some reason).

@lambdageek lambdageek added this to the 6.0.0 milestone Apr 21, 2021
@lambdageek lambdageek removed the untriaged New issue has not been triaged by the area owner label Apr 21, 2021
@SamMonoRT
Copy link
Member

@naricc - maybe tackle this next week.

@BrzVlad BrzVlad assigned BrzVlad and unassigned naricc and SamMonoRT Aug 9, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 10, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 12, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants