From 8a35f4b49a4c4f952b44d56f8bb0b8e4a65f424a Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Wed, 29 Apr 2026 10:09:58 -0700 Subject: [PATCH 1/2] fix(windows): Acquire and release DCs per grab() call (#509) Move GetWindowDC(0) + CreateCompatibleDC out of __init__ and into each grab() call, releasing them in a try/finally before returning. Benefits: - MSS() construction no longer calls GetWindowDC(0), so sct.monitors works even when the desktop DC is unavailable (locked screen, UAC, RDP disconnect, GDI handle pressure) - DCs are acquired and released on the same thread within the same call, eliminating the cross-thread ReleaseDC failures seen in CI Fixes #509. --- src/mss/windows/gdi.py | 108 +++++++++++++++----------------- src/tests/bench_grab_windows.py | 44 +++++++------ src/tests/test_windows.py | 19 ++++++ 3 files changed, 95 insertions(+), 76 deletions(-) diff --git a/src/mss/windows/gdi.py b/src/mss/windows/gdi.py index 02f5ba62..7e2edb86 100644 --- a/src/mss/windows/gdi.py +++ b/src/mss/windows/gdi.py @@ -205,9 +205,7 @@ class MSSImplGdi(MSSImplementation): "_dib", "_dib_array", "_dib_bits", - "_memdc", "_region_width_height", - "_srcdc", "gdi32", "user32", } @@ -226,8 +224,6 @@ def __init__(self) -> None: self._dib: HBITMAP | None = None self._dib_bits: LPVOID = LPVOID() # Pointer to DIB pixel data self._dib_array: ctypes.Array[ctypes.c_char] | None = None # Cached array view of DIB memory - self._srcdc = self.user32.GetWindowDC(0) - self._memdc = self.gdi32.CreateCompatibleDC(self._srcdc) bmi = BITMAPINFO() bmi.bmiHeader.biSize = ctypes.sizeof(BITMAPINFOHEADER) @@ -243,19 +239,10 @@ def __init__(self) -> None: self._bmi = bmi def close(self) -> None: - # Clean-up if self._dib: self.gdi32.DeleteObject(self._dib) self._dib = None - if self._memdc: - self.gdi32.DeleteDC(self._memdc) - self._memdc = None - - if self._srcdc: - self.user32.ReleaseDC(0, self._srcdc) - self._srcdc = None - def _set_cfunctions(self) -> None: """Set all ctypes functions and attach them to attributes.""" cfactory = self._cfactory @@ -357,9 +344,10 @@ def callback(hmonitor: HMONITOR, _data: HDC, rect: LPRECT, _dc: LPARAM) -> bool: def grab(self, monitor: Monitor, /) -> bytearray: """Retrieve all pixels from a monitor using CreateDIBSection. - CreateDIBSection creates a DIB with system-managed memory backing, - allowing BitBlt to write directly to memory we can read. This eliminates - the need for a separate GetDIBits call. + Device contexts (srcdc / memdc) are acquired and released within each + call. This avoids holding GDI resources across threads and allows + ``MSS()`` construction to succeed even when ``GetWindowDC(0)`` would + fail (locked screen, UAC, RDP). See issue #509. Note on biHeight: A bottom-up DIB is specified by setting the height to a positive number, while a top-down DIB is specified by setting the height @@ -367,47 +355,55 @@ def grab(self, monitor: Monitor, /) -> bytearray: https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapinfoheader https://learn.microsoft.com/en-us/windows/win32/api/wingdi/nf-wingdi-createdibsection """ - srcdc, memdc = self._srcdc, self._memdc gdi = self.gdi32 - width, height = monitor["width"], monitor["height"] - - if self._region_width_height != (width, height): - self._region_width_height = (width, height) - self._bmi.bmiHeader.biWidth = width - self._bmi.bmiHeader.biHeight = -height # Negative for top-down DIB - - if self._dib: - gdi.DeleteObject(self._dib) - self._dib = None - - # CreateDIBSection creates the DIB and returns a pointer to the pixel data - self._dib_bits = LPVOID() - self._dib = gdi.CreateDIBSection( - memdc, - self._bmi, - DIB_RGB_COLORS, - ctypes.byref(self._dib_bits), - None, # hSection = NULL (system allocates memory) - 0, # offset = 0 - ) - gdi.SelectObject(memdc, self._dib) - - # Create a ctypes array type that maps directly to the DIB memory. - # This avoids the overhead of ctypes.string_at() creating an intermediate bytes object. - size = width * height * 4 - array_type = ctypes.c_char * size - self._dib_array = ctypes.cast(self._dib_bits, POINTER(array_type)).contents - - # BitBlt copies screen content directly into the DIB's memory - gdi.BitBlt(memdc, 0, 0, width, height, srcdc, monitor["left"], monitor["top"], SRCCOPY | CAPTUREBLT) - - # Flush GDI operations to ensure DIB memory is fully updated before reading. - # This ensures the BitBlt has completed before we access the memory. - gdi.GdiFlush() - - # Read directly from DIB memory via the cached array view - assert self._dib_array is not None # noqa: S101 for type checker - return bytearray(self._dib_array) + srcdc = self.user32.GetWindowDC(0) + try: + memdc = gdi.CreateCompatibleDC(srcdc) + try: + width, height = monitor["width"], monitor["height"] + + if self._region_width_height != (width, height): + self._region_width_height = (width, height) + self._bmi.bmiHeader.biWidth = width + self._bmi.bmiHeader.biHeight = -height # Negative for top-down DIB + + if self._dib: + gdi.DeleteObject(self._dib) + self._dib = None + + # CreateDIBSection creates the DIB and returns a pointer to the pixel data + self._dib_bits = LPVOID() + self._dib = gdi.CreateDIBSection( + memdc, + self._bmi, + DIB_RGB_COLORS, + ctypes.byref(self._dib_bits), + None, # hSection = NULL (system allocates memory) + 0, # offset = 0 + ) + + # Create a ctypes array type that maps directly to the DIB memory. + # This avoids the overhead of ctypes.string_at() creating an intermediate bytes object. + size = width * height * 4 + array_type = ctypes.c_char * size + self._dib_array = ctypes.cast(self._dib_bits, POINTER(array_type)).contents + + # Select the cached DIB into the per-call memdc so BitBlt writes into it. + gdi.SelectObject(memdc, self._dib) + + # BitBlt copies screen content directly into the DIB's memory + gdi.BitBlt(memdc, 0, 0, width, height, srcdc, monitor["left"], monitor["top"], SRCCOPY | CAPTUREBLT) + + # Flush GDI operations to ensure DIB memory is fully updated before reading. + gdi.GdiFlush() + + # Read directly from DIB memory via the cached array view + assert self._dib_array is not None # noqa: S101 for type checker + return bytearray(self._dib_array) + finally: + gdi.DeleteDC(memdc) + finally: + self.user32.ReleaseDC(0, srcdc) def cursor(self) -> None: """Retrieve all cursor data. Pixels have to be RGB.""" diff --git a/src/tests/bench_grab_windows.py b/src/tests/bench_grab_windows.py index b27f8c09..c421e62a 100644 --- a/src/tests/bench_grab_windows.py +++ b/src/tests/bench_grab_windows.py @@ -109,36 +109,40 @@ def benchmark_raw_bitblt() -> None: srccopy = 0x00CC0020 captureblt = 0x40000000 + user32 = ctypes.WinDLL("user32", use_last_error=True) + with mss.MSS() as sct: monitor = sct.monitors[1] width, height = monitor["width"], monitor["height"] left, top = monitor["left"], monitor["top"] - # Force region setup - sct.grab(monitor) - - assert isinstance(sct._impl, mss.windows.gdi.MSSImplGdi) - srcdc = sct._impl._srcdc - memdc = sct._impl._memdc + # Acquire DCs directly for raw benchmarking (the impl no longer + # holds them as instance state — they are per-grab now). + srcdc = user32.GetWindowDC(0) + memdc = gdi32.CreateCompatibleDC(srcdc) print(f"Raw BitBlt benchmark ({width}x{height})") print("=" * 50) - # Test with CAPTUREBLT - start = perf_counter() - for _ in range(ITERATIONS): - bitblt(memdc, 0, 0, width, height, srcdc, left, top, srccopy | captureblt) - gdiflush() - elapsed = perf_counter() - start - print(f"With CAPTUREBLT: {elapsed / ITERATIONS * 1000:.2f}ms ({ITERATIONS / elapsed:.1f} FPS)") + try: + # Test with CAPTUREBLT + start = perf_counter() + for _ in range(ITERATIONS): + bitblt(memdc, 0, 0, width, height, srcdc, left, top, srccopy | captureblt) + gdiflush() + elapsed = perf_counter() - start + print(f"With CAPTUREBLT: {elapsed / ITERATIONS * 1000:.2f}ms ({ITERATIONS / elapsed:.1f} FPS)") - # Test without CAPTUREBLT - start = perf_counter() - for _ in range(ITERATIONS): - bitblt(memdc, 0, 0, width, height, srcdc, left, top, srccopy) - gdiflush() - elapsed = perf_counter() - start - print(f"Without CAPTUREBLT: {elapsed / ITERATIONS * 1000:.2f}ms ({ITERATIONS / elapsed:.1f} FPS)") + # Test without CAPTUREBLT + start = perf_counter() + for _ in range(ITERATIONS): + bitblt(memdc, 0, 0, width, height, srcdc, left, top, srccopy) + gdiflush() + elapsed = perf_counter() - start + print(f"Without CAPTUREBLT: {elapsed / ITERATIONS * 1000:.2f}ms ({ITERATIONS / elapsed:.1f} FPS)") + finally: + gdi32.DeleteDC(memdc) + user32.ReleaseDC(0, srcdc) def analyze_frame_timing() -> None: diff --git a/src/tests/test_windows.py b/src/tests/test_windows.py index 612a98f9..2783697f 100644 --- a/src/tests/test_windows.py +++ b/src/tests/test_windows.py @@ -145,6 +145,25 @@ def test_region_not_caching() -> None: assert dib1 != dib2 +def test_monitors_without_grab() -> None: + """Regression test for issue #509. + + Constructing ``MSS`` and reading ``sct.monitors`` must not call + ``GetWindowDC(0)``. Device contexts are only needed for ``grab()``. + """ + with mss.MSS() as sct: + impl = sct._impl + assert isinstance(impl, MSSImplGdi) + + # DCs are no longer instance attributes. + assert not hasattr(impl, "_srcdc") + assert not hasattr(impl, "_memdc") + + monitors = sct.monitors + assert len(monitors) >= 1 + assert "width" in monitors[0] + + def run_child_thread(loops: int) -> None: for _ in range(loops): with mss.MSS() as sct: # New sct for every loop From 7fc867e16ceed2fd8e03e2409b3788ebeb824cef Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Wed, 29 Apr 2026 11:10:57 -0700 Subject: [PATCH 2/2] fix(windows): Address review feedback for #516 - Rewrite regression test to simulate GetWindowDC failure and verify monitors still work while grab() properly raises ScreenShotError - Add changelog entry to v11.0.0.md --- docs/source/release-history/v11.0.0.md | 2 ++ src/tests/test_windows.py | 27 ++++++++++++++++---------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/docs/source/release-history/v11.0.0.md b/docs/source/release-history/v11.0.0.md index 9f264933..7a13ef06 100644 --- a/docs/source/release-history/v11.0.0.md +++ b/docs/source/release-history/v11.0.0.md @@ -18,6 +18,8 @@ The MSS project has chosen to end support for Python 3.9, in order to focus our Improved error handling when interacting with Win32 API, which will improve diagnostics of issues. +Device contexts are now acquired and released within each `grab()` call, allowing monitor enumeration to work even when `GetWindowDC(0)` fails (#509). + ### General Improvements The MSS context object will now always surface inner exceptions, even if `__exit__` may also generate an exception during tear-down. diff --git a/src/tests/test_windows.py b/src/tests/test_windows.py index 2783697f..7f6061d6 100644 --- a/src/tests/test_windows.py +++ b/src/tests/test_windows.py @@ -145,23 +145,30 @@ def test_region_not_caching() -> None: assert dib1 != dib2 -def test_monitors_without_grab() -> None: +def test_monitors_work_when_getwindowdc_fails() -> None: """Regression test for issue #509. - Constructing ``MSS`` and reading ``sct.monitors`` must not call - ``GetWindowDC(0)``. Device contexts are only needed for ``grab()``. + ``GetWindowDC(0)`` can fail (locked screen, UAC, RDP, GDI pressure). + Enumerating monitors must still work because it only needs + ``EnumDisplayMonitors`` / ``GetMonitorInfoW``. """ with mss.MSS() as sct: impl = sct._impl assert isinstance(impl, MSSImplGdi) - # DCs are no longer instance attributes. - assert not hasattr(impl, "_srcdc") - assert not hasattr(impl, "_memdc") - - monitors = sct.monitors - assert len(monitors) >= 1 - assert "width" in monitors[0] + # Simulate GetWindowDC failing — grab() should raise, but + # monitors must remain accessible. + original = impl.user32.GetWindowDC + impl.user32.GetWindowDC = lambda _hwnd: 0 # type: ignore[attr-defined] + try: + monitors = sct.monitors + assert len(monitors) >= 1 + assert "width" in monitors[0] + + with pytest.raises(ScreenShotError): + sct.grab(monitors[1]) + finally: + impl.user32.GetWindowDC = original # type: ignore[attr-defined] def run_child_thread(loops: int) -> None: