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: https://github.com/nvaccess/nvda/blob/1cf36fbe2e0a7b40cccea707914fe575b92c54b6/source/winKernel.py#L77

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

Doing something like the following could resolve that: https://github.com/nvaccess/nvda/blob/45cb098b2f6238ae875b42ee2f2b32b4912f2366/nvdaHelper/eventHandler/EventHandlerDllWrapper.py#L27

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:

https://github.com/nvaccess/nvda/blob/1cf36fbe2e0a7b40cccea707914fe575b92c54b6/source/winKernel.py#L77

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:

https://github.com/nvaccess/nvda/blob/45cb098b2f6238ae875b42ee2f2b32b4912f2366/nvdaHelper/eventHandler/EventHandlerDllWrapper.py#L27

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: https://github.com/leonardder/nvda/commit/3e8bfa03aaac563627afb796ddcf61c02565308a

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.

[LeonarddeR]

I expanded the prototype, so to see what it looks like, it's probably best to refer to the ctypesHelper branch I'm working on. It also contains an example implementation in winKernel for createWaitableTimer. Type hints should be available for type checkers as well as at runtime using typing.get_type_hints and inspect.signature. Thanks to the magic of functools.update_wrapper

[feerrenrut]

It looks like there is enough logic and conditions in there that it would be worth writing unit tests. As a bonus it will help to document how it works.

[feerrenrut]

Did you notice the note in the docs for functools.wraps?

Without the use of this decorator factory, the name of the example function would have been 'wrapper', and the docstring of the original example() would have been lost.

Is this relevant to your prototype?

[LeonarddeR]

Did you notice the note in the docs for functools.wraps?

Without the use of this decorator factory, the name of the example function would have been 'wrapper', and the docstring of the original example() would have been lost.

Is this relevant to your prototype?

Yes, probably, thanks for pointing me at this. I might have to do some additional magic under the hood to make docstrings work. The problem is that functools.update_wrapper actually expects a real FunctionType. Therefore, to make doc strings work, they need to be set on the class.

It looks like there is enough logic and conditions in there that it would be worth writing unit tests. As a bonus it will help to document how it works.

If you believe this prototype is the right way to go, unit tests are certainly planned.

[feerrenrut]

If you believe this prototype is the right way to go

This is hard to say without a few more examples of how it is used. I think tests will help with that. It would also be good to at least outline alternative approaches and compare the benefits of each.

[LeonarddeR]

I think there are some alternative approaches:

1. Set argtypes and restype on a per _FuncPtr object basis, and leave the wrappers as they are. However, if we don't do this using a helper function like in this prototype, we don't have type annotations.
2. Instead of making the decorator return the WINFUNCTYPE object, return another wrapper function taking *args, **kwargs, and let that wrapper forward the arguments to the winfunctype. This way, we could easily preserve doc string and other attributes, like name, qualname and the repr method. Validation would still be performed by the winfunctype object. I think I"m going to explore this approach, as it might be a bit cleaner than patching the winfunctype.
3. Don't create a WINFUNCTYPE object, but wrap the _FuncPtr object directly. This requires patching the _FuncPtr objects, and therefore lets the winKernel module patch function pointers on winKernel.kernel32. I consider this a bit dirty, I really prefer the WINFUNCTYPE approach myself.

[LeonarddeR]

@josephsl curious to know what you think about this.

I also found out that python 3.8 adds functions to the typing module to inspect types, typing.get_origin and typing.get_args. That would allow us to use Generics, like this

CType = TypeVar("CType")

class OutParam(Generic[CType]):
    ...

def GetClientRect(hwnd: ctypes.wintypes.HWND, rect: OutParam[ctypes.wintypes.LPRECT) -> ctypes.wintypes.BOOL:
    ...

All annotations for which typing.get_origin returns OutParam are output parameters. The type to use for the output parameter could be retrieved with typing.get_args

[josephsl]

Hi, was this tested under Python 3.8 with a test wrapper module? As for implications of that change, I don’t know as ctypes isn’t really my area. Thanks.

[LeonarddeR]

Hi, was this tested under Python 3.8 with a test wrapper module?

Not yet, this was based on python 3.8 docs.

[Adriani90]

What is now the current status on this, now that we are on Python 3.11? cc: @gerald-hartig is there an official positoin of NV Access on this topic?