IronLanguages / IronLanguages/ironpython3

Incorrect implementation of ctypes.get_last_error and set_last_error

Open
#1,477 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

module-ctypes
Dominant language
C#
Stars
2.8k
Forks
316
Avg merge
1d 9h
Merged PRs (30d)
1

Description

These functions are Windows-specific. The current IronPython implementation uses NativeFunctions.GetLastError() and SetLastError(), which are directly imported from kernel32.dll. Those error codes are global and are affected by other factors, like the CLR internal functions. This implementation not correct: CPython's implementation has errors cached in ctypes (thread-locally). From the CPython documentation:

ctypes.get_last_error()

Windows only: returns the current value of the ctypes-private copy of the system LastError variable in the calling thread.

ctypes.set_last_error(value)

Windows only: set the current value of the ctypes-private copy of the system LastError variable in the calling thread to value and return the previous value.

ctypes.GetLastError()

Windows only: Returns the last error code set by Windows in the calling thread. This function calls the Windows GetLastError() function directly, it does not return the ctypes-private copy of the error code.

.NET recognizes the limitation of directly calling kernel32.dll for error codes and provides a better alternative: Marshall.GetLastWin32Error():

On Windows systems, GetLastWin32Error exposes the Win32 GetLastError function from Kernel32.DLL. This method exists because it is not reliable to make a direct platform invoke call to GetLastError to obtain this information. If you want to access this error code, you must call GetLastWin32Error instead of writing your own platform invoke definition for GetLastError and calling it. The common language runtime can make internal calls to APIs that overwrite the GetLastError maintained by the operating system.

There is a difference in the behavior of the GetLastWin32Error method on .NET Core and .NET Framework when DllImportAttribute.SetLastError is true. On .NET Framework, the GetLastWin32Error method can retain error information from one P/Invoke call to the next. On .NET Core, error information is cleared before P/Invoke call, and the GetLastWin32Error represents only error information from the last method call.

On .NET 6 and later versions, this method is functionally equivalent to GetLastPInvokeError, which is named to better reflect the intent of the API and its cross-platform nature. GetLastPInvokeError should be preferred over GetLastWin32Error.

As for setting errors, .NET 6 has SetLastPInvokeError (complementing GetLastPInvokeError) and SetLastSystemError (thread locally). I see nothing specific for earlier .NET versions.

It looks like on .NET 6+, get_last_error is (roughly) equivalent to GetLastPInvokeError, and set_last_error is a sequence of calls SetLastSystemError and SetLastPInvokeError. On other platforms, this functionality may need to be emulated. ctypes.GetLastError (currently missing) seems to be closest to Marshal.GetLastWin32Error (with all the caveats on .NET Core).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the ctypes implementation that calls NativeFunctions.GetLastError() and SetLastError(), then compare its behavior with CPython's documented ctypes-private error handling. Review the .NET alternatives named in the issue, including GetLastWin32Error, GetLastPInvokeError, SetLastPInvokeError, and SetLastSystemError; done means the Windows APIs match the documented ctypes semantics, including the missing ctypes.GetLastError behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, python
Domain
operating-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.