Make winKernel / winUser wrapper functions type safe and more reliable

[LeonarddeR]

Is your feature request related to a problem? Please describe.

NVDA is using many windows API functions that are c function and therefore require certain types as input / output parameters. Most of them can be found in the winKernel and winUser modules.
However, the functions we're using are merely wrappers of these functions, without checking what types we're passing at them and getting from them.

By default, ctypes assumes a return type of ctypes.c_long for all library functions loaded with the WinDLL library loader. However, many of these functions return other types, such as DWORD (unsigned long) and HANDLE. Furthermore, when throwing parameters at these functions, ctypes by default only knows how many bytes the function expects as its input. This is all in depth explained in the ctypes docs

Example:

>>> winKernel.closeHandle()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "winKernel.pyc", line 134, in closeHandle
ValueError: Procedure probably called with not enough arguments (4 bytes missing)
>>> winKernel.closeHandle("a")
0

As you can see, you can throw an empty string at this function. ctypes will convert the string into a c_wchar_p which is 4 bytes in size, the same size as a 32 bit handle. The function fails, but the function shouldn't have been executed at all.

Describe the solution you'd like

I propose two things:

1. Define a standard for new wrappers added to winKernel and winUser
2. Update the current wrappers according to this new standard.

As a standard, I'd propose to no longer wrap these functions with python def style functions, but using ctypes.WINFUNCTYPE. This allows us to:

• Remove a wrapping layer, as ctypes.WINFUNCTYPE is a way to annotate the function pointers
• Provide expected types, names and default values for arguments
• Automatic Windows error handling/raising (GetLastError)
• Automatic input parameters for functions like user32.GetClientRect. You set RECT as an input parameter and ctypes automatically passes an empty structure to the function

A downside of this approach might be that we don't have type annotations.

[feerrenrut]

An example function:

nvda/source/winKernel.py

Line 77 in 1cf36fb

def createWaitableTimer(securityAttributes=None, manualReset=False, name=None):

A downside of this approach might be that we don't have type annotations.

Doing something like the following could resolve that:

nvda/nvdaHelper/eventHandler/EventHandlerDllWrapper.py

Line 27 in 45cb098

class EventHandlerDll:

Taking this approach also means that we can attach docs to these functions. Getting type info and docs at the call site is very helpful.

[LeonarddeR]

An example function:

nvda/source/winKernel.py

Line 77 in 1cf36fb

def createWaitableTimer(securityAttributes=None, manualReset=False, name=None):

How do you mean this reference? I wrote this function myself, but it still hasn't type checking at the ctypes level, so you're still allowed to throw things at that function that might break.

I will update the initial description with some additional examples and justifications.

[LeonarddeR]

Doing something like the following could resolve that:

nvda/nvdaHelper/eventHandler/EventHandlerDllWrapper.py

Line 27 in 45cb098

class EventHandlerDll:

I'm not following how this example is supposed to work. getEventHandlerDll is supposed to return a EventHandlerDll instance, but it will return an instance of ctypes.CDLL. Probably better to discuss this in #10556, though.

I was more thinking about an approach like visionEnhancementProviders.screenCurtain.Magnification

[LeonarddeR]

I actually found a serious bug that's caused by this inaccurateness.

the return type of WaitForSingleObject is DWORD. If its value is WAIT_FAILED = 0xffffffff, the function raises a Windows Error. However, this is never the case, as the return type is set to its default, ctypes.c_int, which is unsigned. Therefore the result of WaitForSingleObject will never be treated as WAIT_FAILED at all, as that's out of the range of c_int.

If you add kernel32.WaitForSingleObject.restype = ctypes.wintypes.DWORD to winKernel, you will notice that NVDA will not start at all. If you do it at runtime, you get tracebacks instantly.

I'm afraid there are more ugly bugs like this.

[feerrenrut]

I'm not following how this example is supposed to work.

How you describe it is how it's supposed to work. The type information is provided for IDE's and automatic type checkers. The types of EventHandlerDll and the returned ctypes.CDLL match. We could probably create an annotation that added these types to the CDLL methods automatically.

[feerrenrut]

an example function

so you're still allowed to throw things at that function that might break.

I wanted to point to an example in the code base that does not have the types set.

[LeonarddeR]

How you describe it is how it's supposed to work. The type information is provided for IDE's and automatic type checkers. The types of EventHandlerDll and the returned ctypes.CDLL match. We could probably create an annotation that added these types to the CDLL methods automatically.

I think that would be required for runtime inspection as well. Creating a class only for a type checker to be satisfied feels a bit tricky to me.

[feerrenrut]

It's just a way of specifying the type returned by the function. I'm not aware of another way to do it?

[LeonarddeR]

How about the following approach I'd like to explore:

1. Define the functions as in your example

2. Create a new ctypesHelper module

3. In that module, provide a decorator that does the following:

   • Discard the body of the decorated function, it will be empty
   • Inspect the function for type annotations
   • Construct the proper WINFUNCTYPE object using the annotated function, along with default values
   • We could subclass typing.Generic to create an annotation for out params

[feerrenrut]

Yep, that's what I had in mind, but not confident it is possible haha!

We could subclass typing.Generic to create an annotation for out params

I'm not sure what you mean by this.

[LeonarddeR]

I wrote a prototype:
LeonarddeR@3e8bfa0

It can be used as follows:

import ctypes, ctypesHelper, winKernel

@ctypesHelper.annotatedCFunction(winKernel.kernel32)
def CloseHandle(handle: ctypes.wintypes.HANDLE) -> ctypes.wintypes.BOOL:
	...

Runtime inspection of type hints is not possible.

>>> typing.get_type_hints(CloseHandle)
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "C:\Python37\lib\typing.py", line 1002, in get_type_hints
    'or function.'.format(obj))
TypeError: <WinFunctionType object at 0x0492B300> is not a module, class, method, or function.

We could subclass typing.Generic to create an annotation for out params

I'm not sure what you mean by this.

I thought you needed to use typing.Generic to construct your own type like typing.Optional, but turns out I was wrong. feel free to forget this.