From 2f52bfbd9388e76e30593b7dbc6824d520b4fa9f Mon Sep 17 00:00:00 2001 From: Joel Ray Holveck Date: Tue, 27 Jan 2026 16:20:29 -0800 Subject: [PATCH 1/2] Bug fix: KeyboardInterrupt while copying data from an mmapped region In the XShmGetImage backend, there was a window of time during which a KeyboardInterrupt (or other asynchronous exception) would cause cleanup of the MSS object to raise a different exception. This is one of the slower parts of this backend, so it's a time when asynchronous exceptions can hit pretty easily. While a Python library almost never can guarantee correctness following an asynchronous exception, ending a program with Ctrl-C is not uncommon. We can't guarantee correctness in all circumstances, and shouldn't try to. All this patch does is to avoid the exception that may be hard for an end user to understand. Instead, the top-level user code will now see the KeyboardInterrupt, and not the seemingly-unrelated cleanup exception. --- src/mss/linux/xshmgetimage.py | 15 +++++++++++++-- src/tests/test_gnu_linux.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/mss/linux/xshmgetimage.py b/src/mss/linux/xshmgetimage.py index 9858552..5628910 100644 --- a/src/mss/linux/xshmgetimage.py +++ b/src/mss/linux/xshmgetimage.py @@ -190,8 +190,19 @@ def _grab_impl_xshmgetimage(self, monitor: Monitor) -> ScreenShot: new_size = monitor["width"] * monitor["height"] * 4 # Slicing the memoryview creates a new memoryview that points to the relevant subregion. Making this and # then copying it into a fresh bytearray is much faster than slicing the mmap object. - img_mv = memoryview(self._buf)[:new_size] - img_data = bytearray(img_mv) + try: + img_mv: memoryview | None = memoryview(self._buf)[:new_size] + assert img_mv is not None # noqa: S101 + img_data = bytearray(img_mv) + finally: + # Imagine that an exception happened in the above code, such as an asynchronous KeyboardInterrupt. Let's + # imagine it happened after we created img_mv, but while we were populating img_data, a process that can + # take a few milliseconds. That exception includes this stack frame, and hence holds a reference to + # img_mv. If the exception unwinds to the enclosing `with mss() as sct:` block, then self._buf.close() + # would be executed, to close the mmapped region. But img_mv still exists, and self._buf.close() would + # throw an exception, because it can't close the region while references exist. To prevent that, remove + # the reference to img_mv during the stack unwind here. + img_mv = None return self.cls_image(img_data, monitor) diff --git a/src/tests/test_gnu_linux.py b/src/tests/test_gnu_linux.py index dcb1d99..048d6f3 100644 --- a/src/tests/test_gnu_linux.py +++ b/src/tests/test_gnu_linux.py @@ -4,6 +4,7 @@ from __future__ import annotations +import builtins import ctypes.util import platform from ctypes import CFUNCTYPE, POINTER, _Pointer, c_int @@ -315,3 +316,32 @@ def test_shm_fallback() -> None: sct.grab(sct.monitors[0]) # Ensure that it really did have to fall back; otherwise, we'd need to change how we test this case. assert sct.shm_status == mss.linux.xshmgetimage.ShmStatus.UNAVAILABLE + + +def test_exception_while_holding_memoryview(monkeypatch: pytest.MonkeyPatch) -> None: + """Verify that an exception at a particular point doesn't prevent cleanup. + + The particular point is the window when the XShmGetImage's mmapped + buffer has a memoryview still outstanding, and the pixel data is + being copied into a bytearray. This can take a few milliseconds. + """ + # Force an exception during bytearray(img_mv) + real_bytearray = builtins.bytearray + + def boom(*args: list, **kwargs: dict[str, Any]) -> bytearray: + # Only explode when called with the memoryview (the code path we care about). + if len(args) > 0 and isinstance(args[0], memoryview): + # We still need to eliminate args from the stack frame, just like the fix. + del args, kwargs + msg = "Boom!" + raise RuntimeError(msg) + return real_bytearray(*args, **kwargs) + + # We have to be careful about the order in which we catch things. If we were to catch and discard the exception + # before the MSS object closes, it won't trigger the bug. That's why we have the pytest.raises outside the + # mss.mss block. In addition, we do as much as we can before patching bytearray, to limit its scope. + with pytest.raises(RuntimeError, match="Boom!"), mss.mss(backend="xshmgetimage") as sct: # noqa: PT012 + monitor = sct.monitors[0] + with monkeypatch.context() as m: + m.setattr(builtins, "bytearray", boom) + sct.grab(monitor) From a1b7499a4176fcf96fa6332d48651373596610f4 Mon Sep 17 00:00:00 2001 From: Joel Ray Holveck Date: Tue, 27 Jan 2026 22:59:35 -0800 Subject: [PATCH 2/2] Switch to a simpler implementation I just learned that memoryviews can be used as context managers, and release their buffers at the end of that, instead of having to be GC'd. --- src/mss/linux/xshmgetimage.py | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/src/mss/linux/xshmgetimage.py b/src/mss/linux/xshmgetimage.py index 5628910..cdd882c 100644 --- a/src/mss/linux/xshmgetimage.py +++ b/src/mss/linux/xshmgetimage.py @@ -188,21 +188,12 @@ def _grab_impl_xshmgetimage(self, monitor: Monitor) -> ScreenShot: # Snapshot the buffer into new bytearray. new_size = monitor["width"] * monitor["height"] * 4 - # Slicing the memoryview creates a new memoryview that points to the relevant subregion. Making this and - # then copying it into a fresh bytearray is much faster than slicing the mmap object. - try: - img_mv: memoryview | None = memoryview(self._buf)[:new_size] - assert img_mv is not None # noqa: S101 - img_data = bytearray(img_mv) - finally: - # Imagine that an exception happened in the above code, such as an asynchronous KeyboardInterrupt. Let's - # imagine it happened after we created img_mv, but while we were populating img_data, a process that can - # take a few milliseconds. That exception includes this stack frame, and hence holds a reference to - # img_mv. If the exception unwinds to the enclosing `with mss() as sct:` block, then self._buf.close() - # would be executed, to close the mmapped region. But img_mv still exists, and self._buf.close() would - # throw an exception, because it can't close the region while references exist. To prevent that, remove - # the reference to img_mv during the stack unwind here. - img_mv = None + # Slicing the memoryview creates a new memoryview that points to the relevant subregion. Making this and then + # copying it into a fresh bytearray is much faster than slicing the mmap object. Make sure we don't hold an + # open memoryview if an exception happens, since that will prevent us from closing self._buf during the stack + # unwind. + with memoryview(self._buf) as img_mv: + img_data = bytearray(img_mv[:new_size]) return self.cls_image(img_data, monitor)