screeninfo icon indicating copy to clipboard operation
screeninfo copied to clipboard

fix 'No enumerators available' - on Windows 10

Open feerrenrut opened this issue 5 years ago • 32 comments

Fixes #30

dc_full is a pointer, if interpretted as a signed number may it be negative despite the call to GetDC being successful.

Failure is indicated by NULL -> 0. This PR sets the return type for the foriegn function GetDC so the value can be compared to None.

  • See GetDC docs: https://docs.microsoft.com/en-gb/windows/win32/api/winuser/nf-winuser-getdc
  • See Windows types: https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types

ReleaseDC was not being called with enough arguments, it should also take the same Window Handle (HWND) as GetDC.

  • See ReleaseDC docs: https://docs.microsoft.com/en-gb/windows/win32/api/winuser/nf-winuser-releasedc

feerrenrut avatar Apr 13 '20 11:04 feerrenrut

Pinging @hhannine for a review, as it contains his code.

rr- avatar Apr 13 '20 13:04 rr-

I probably won't put anymore effort into this PR. Please make modifications as you see fit, or close it.

feerrenrut avatar Apr 13 '20 15:04 feerrenrut

Thanks for honest answer

rr- avatar Apr 13 '20 15:04 rr-

I've now gotten a couple more physical size detection failures, I wonder what causes them.. It goes through consistently on the first try though.

Edit: Resolution detection has worked correctly every single time however.

hhannine avatar Apr 13 '20 18:04 hhannine

@hhannine could we rework the loop to cater for the new findings?

rr- avatar Apr 13 '20 18:04 rr-

I need to catch some intermediate step data to see if I can find if anything is different when the detection fails. Couldn't get anything yet.

hhannine avatar Apr 13 '20 20:04 hhannine

When the size detection fails I'm getting dc_full values

18446744073424413210
18446744071729909776
18446744072803654137

and when it succeeds they're more like

2063672708
721495611

Is this size discrepancy a sign issue and this is again the failure with negative dc_full values?

For whatever reason I'm getting failures at a higher rate now than I got yesterday.

hhannine avatar Apr 14 '20 06:04 hhannine

Those values suspiciously all start with the same digits, are also larger than a 32 bit void pointer (max 4 bytes). To me this indicates memory corruption. The first thing I would do is confirm the types for all the dll functions, and specify them according to ctypes documentation on foreign functions

When you say "size detection fails" do you mean the calls to GetMonitorInfoW and GetDeviceCaps? How are you determining the error? Is there an exception?

https://docs.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getmonitorinfow https://docs.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-getdevicecaps https://docs.microsoft.com/en-gb/windows/win32/api/winuser/nf-winuser-getdc https://docs.microsoft.com/en-gb/windows/win32/api/winuser/nf-winuser-releasedc

feerrenrut avatar Apr 14 '20 15:04 feerrenrut

Thanks for taking a look. I'm having trouble debugging this further: no exceptions are raised so the only output I get of the failure is that the physical sizes and display names are all None. And as I try to add some prints to tell me what the data are none of the prints come through when this failure occurs, only when the enumeration is successful. Even a print on the first line of the callback is not printed if the failure occurs and yet it returns the resolutions..

Any ideas what is going on? Would it not return anything I'd believe that the call to EnumDisplayMonitors fails based on the data it's getting but since there's some output I don't know what to make of this.

I also noticed that 2^64 = 18446744073709551616‬, notably close to the strange values I listed above. Is this a typing issue for dc_full?

hhannine avatar Apr 15 '20 07:04 hhannine

I added typing to all the dll function calls. While doing this I found that the types specified for MonitorEnumProc were quite wrong.

feerrenrut avatar Apr 15 '20 19:04 feerrenrut

hwnd = HDC

Good catch, I'll fix that.

pass None to EnumDisplayMonitors

I hoped that ctypes would complain if you gave it the wrong type (now that argtypes are specified) but I just tested and this doesn't seem to be the case. To be sure I will replace those literals with the appropriate typed constructor.

feerrenrut avatar Apr 15 '20 19:04 feerrenrut

Thanks for taking the time to make things rigorous! Unfortunately however, I'm not getting any physical sizes or display names at all. First line print from the callback is never printed.

hhannine avatar Apr 15 '20 20:04 hhannine

However now that I changed LPRECT(None) back to None I'm getting good values. I kept the new LPARAM(0).

Edit: However, I'm now getting a randomly occuring new exception:

Traceback (most recent call last):
  File "_ctypes/callbacks.c", line 237, in 'calling callback function'
  File "C:\Users\hhan\Documents\GitHub\screeninfo\screeninfo\enumerators\windows.py", line 82, in callback
    h_size = GetDeviceCaps(dc, HORZSIZE)
ctypes.ArgumentError: argument 1: <class 'OverflowError'>: int too long to convert

hhannine avatar Apr 15 '20 20:04 hhannine

And with that fix, how many loop iterations do you need to get the good values?

rr- avatar Apr 15 '20 20:04 rr-

I ran into a new exception that I added above, and it now fails the enumeration of that specific display that it occurs on and I'm getting an incomplete list on monitors as an output.

hhannine avatar Apr 15 '20 20:04 hhannine

Now after some testing I got again a dc_full = 18446744072350667922 and the enumeration only returned resolutions.

hhannine avatar Apr 15 '20 20:04 hhannine

It seems that now with the new changes it is possible to get bad dc values in the callback even though the original dc_full was sensible. Here both callback calls get a bad dc and raise separate exceptions, leading to empty monitor list:

>>> gm()
dc_full 1577129041
init callb
dc 18446744073155974335
Traceback (most recent call last):
  File "_ctypes/callbacks.c", line 237, in 'calling callback function'
  File "C:\Users\hhan\Documents\GitHub\screeninfo\screeninfo\enumerators\windows.py", line 82, in callback
    h_size = GetDeviceCaps(dc, HORZSIZE)
ctypes.ArgumentError: argument 1: <class 'OverflowError'>: int too long to convert
init callb
dc 18446744073290188851
Traceback (most recent call last):
  File "_ctypes/callbacks.c", line 237, in 'calling callback function'
  File "C:\Users\hhan\Documents\GitHub\screeninfo\screeninfo\enumerators\windows.py", line 82, in callback
    h_size = GetDeviceCaps(dc, HORZSIZE)
ctypes.ArgumentError: argument 1: <class 'OverflowError'>: int too long to convert

hhannine avatar Apr 15 '20 20:04 hhannine

Can you print out the type of dc inside the callback. I'm wondering if the callback is being given a value that needs to be converted / interpreted properly before use.

feerrenrut avatar Apr 16 '20 10:04 feerrenrut

With type(dc) I get for both dc_full and dc that

dc_full 486610913 type: <class 'int'>

dc 654382316 type: <class 'int'>

dc 18446744072971424825 type: <class 'int'>

Does this help or did you want me to do something ctypes specific?

hhannine avatar Apr 16 '20 20:04 hhannine

Does this help or did you want me to do something ctypes specific?

It didn't give me any leads. I can't reproduce the issue that you're seeing in either python 3.7.3 or 3.8.2. I have fixed the issue with LPRECT(None) the docs for EnumDisplayMonitors refer to LPCRECT rather than LPRECT, it resolves to the same type, but I figure it's best to match the docs.

One thing that is worth trying is moving the creation of the MONITORENUMPROC so that we can be sure it isn't garbage collected during the call to EnumDisplayMonitors hopefully that's what is causing the error. @hhannine Can you test and let me know?

feerrenrut avatar Apr 18 '20 17:04 feerrenrut

How unfortunate, I'm still getting the issue with the new changes. I also updated to python 3.8.2 but that did not help either.

Is this then a ctypes regression or something if you don't have the issue? Hardware specific? Should we reimplement the checking of dc_full for being of moderate size, whatever the spec says it should be instead of a 64-bit integer? As a workaround like we have in the current release?

I was messing around and I think I might have worked around the issue by testing if a dc is larger than 2^32 and if it is, subtracting 2^64 from it, yielding actual output...

dc_full 1577131533
dc 18446744073189526892 <class 'int'>
dc -520024724 <class 'int'>
dc 18446744073004979777 <class 'int'>
dc -704571839 <class 'int'>
[Monitor(x=0, y=0, width=3840, height=2160, width_mm=598, height_mm=336, name='\\\\.\\DISPLAY1'), Monitor(x=3840, y=-181, width=1440, height=2560, width_mm=311, height_mm=553, name='\\\\.\\DISPLAY2')]

hhannine avatar Apr 18 '20 18:04 hhannine

The above workaround does seem to work reliably for dc in the callback but it does not work for dc_full.

hhannine avatar Apr 18 '20 18:04 hhannine

I can't reproduce the issue with bad dc values. I've tried 3.7 32 bit, 3.7 64 bit, 3.8 32 bit.

I added the following to the callback function:

print(f"MonLen {len(monitors)} - got dc {dc}")
        if dc is not None and dc > pow(2, 32):
            # Got a valid DC, break.
            print(f"callback error: {dc}")

and the following to right after GetDC:

print(f"Retry {retry} - got dc_full {dc_full}")
        if dc_full > pow(2, 32):
            # Got a valid DC, break.
            print(f"GetDC error: {dc_full}")

I called enumerate_monitors 100 times in a loop like so:

    for i in range(100):
        for m in enumerate_monitors():
            print(f"{i} - {str(m)}")

I only have one monitor. All values of dc were 4127268177 and all values of dc_full were 1677791573

A few questions that might shed light on this:

  • Which exact version of python are you using, run python -VV to get a string like Python 3.7.3 (v3.7.3:ef4ec6ed12, Mar 25 2019, 21:26:53) [MSC v.1916 32 bit (Intel)]
  • How are you running it this sample (do you use an IDE, or from cmd or something else)?
  • Does the error go away if you use Python 3.7?
  • Try removing the call to GetMonitorInfoW from callback. This is to ensure it isn't corrupting memory. String buffers are always suspicious.
  • If you replace dc_full with None when calling EnumDisplayMonitors, what is the value of dc in callback?
  • In a previous comment you mentioned getting a bad dc_full value, did you mean dc here, or has this problem now been resolved?

It might be a bug in the ctypes library, but without being ale to reproduce the problem it's very difficult to be sure or debug.

feerrenrut avatar Apr 19 '20 12:04 feerrenrut

  1. Python 3.8.2 (tags/v3.8.2:7b3ab59, Feb 25 2020, 23:03:10) [MSC v.1916 64 bit (AMD64)]

  2. I'm running from cmd

  3. todo

  4. Tried removing GetMonitorInfoW call, didn't change the dc values I'm getting.

  5. If I set dc_full = None before the call to EnumDisplayMonitors, the callback is not called, or atleast none of my prints are printed from the callback and I can't tell what the dc values are, if anything.

  6. I did mean dc_full, I was and still am getting similar bad values for both dc_full and dc. Though the likelihood of getting a bad dc_full is much lower so it's a bit hard to test, whereas for almost every call of get_monitors there's at least one if not more bad dc.

hhannine avatar Apr 21 '20 09:04 hhannine

With Python3.7

Python 3.7.4 (tags/v3.7.4:e09359112e, Jul  8 2019, 20:34:20) [MSC v.1916 64 bit (AMD64)] on win32

I got out

dc_full 18446744072350671091 <class 'int'>
[Monitor(x=0, y=0, width=3840, height=2160, width_mm=None, height_mm=None, name=None), Monitor(x=3840, y=-181, width=1440, height=2560, width_mm=None, height_mm=None, name=None)]

No prints from callback there.

When it succeeds:

dc_full 302062927 <class 'int'>
dc 2013338536 <class 'int'>
dc 570496558 <class 'int'>
[Monitor(x=0, y=0, width=3840, height=2160, width_mm=598, height_mm=336, name='\\\\.\\DISPLAY1'), Monitor(x=3840, y=-181, width=1440, height=2560, width_mm=311, height_mm=553, name='\\\\.\\DISPLAY2')]

hhannine avatar Apr 21 '20 09:04 hhannine

Also on

Python 3.7.7 (tags/v3.7.7:d7c567b08f, Mar 10 2020, 10:41:24) [MSC v.1900 64 bit (AMD64)] on win32

I got after a while of testing

dc_full 18446744073692842109 <class 'int'>
[Monitor(x=0, y=0, width=3840, height=2160, width_mm=None, height_mm=None, name=None), Monitor(x=3840, y=-181, width=1440, height=2560, width_mm=None, height_mm=None, name=None)]

No prints from callback or exceptions.

hhannine avatar Apr 21 '20 09:04 hhannine

I'm also getting bad dcs on Python 3.7.7.

hhannine avatar Apr 21 '20 09:04 hhannine

By the way, by writing a loop of calls of get_monitors I'm not getting refreshed dc_full values and a large number of calls can succeed in succession with the same good dc_full. I need to restart python and do calls with some time delay to sometimes get new dc_full values. I've tried also deleting the pycache and it might help getting new dc_full values, maybe not I don't know. Getting a bad dc_full is comparatively rare.

hhannine avatar Apr 21 '20 09:04 hhannine

This fix worked on my windows 10 system.

uspinto avatar Sep 02 '20 15:09 uspinto

To everyone involved, is this PR good to merge?

rr- avatar Oct 23 '20 07:10 rr-

Can you please squash the changes?

rr- avatar Dec 27 '20 20:12 rr-