Skip to content

Commit 9637209

Browse files
authored
feat: improve #467 implementation (#468)
* 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. * 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.
1 parent e5888a7 commit 9637209

1 file changed

Lines changed: 6 additions & 15 deletions

File tree

src/mss/linux/xshmgetimage.py

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -188,21 +188,12 @@ def _grab_impl_xshmgetimage(self, monitor: Monitor) -> ScreenShot:
188188

189189
# Snapshot the buffer into new bytearray.
190190
new_size = monitor["width"] * monitor["height"] * 4
191-
# Slicing the memoryview creates a new memoryview that points to the relevant subregion. Making this and
192-
# then copying it into a fresh bytearray is much faster than slicing the mmap object.
193-
try:
194-
img_mv: memoryview | None = memoryview(self._buf)[:new_size]
195-
assert img_mv is not None # noqa: S101
196-
img_data = bytearray(img_mv)
197-
finally:
198-
# Imagine that an exception happened in the above code, such as an asynchronous KeyboardInterrupt. Let's
199-
# imagine it happened after we created img_mv, but while we were populating img_data, a process that can
200-
# take a few milliseconds. That exception includes this stack frame, and hence holds a reference to
201-
# img_mv. If the exception unwinds to the enclosing `with mss() as sct:` block, then self._buf.close()
202-
# would be executed, to close the mmapped region. But img_mv still exists, and self._buf.close() would
203-
# throw an exception, because it can't close the region while references exist. To prevent that, remove
204-
# the reference to img_mv during the stack unwind here.
205-
img_mv = None
191+
# Slicing the memoryview creates a new memoryview that points to the relevant subregion. Making this and then
192+
# copying it into a fresh bytearray is much faster than slicing the mmap object. Make sure we don't hold an
193+
# open memoryview if an exception happens, since that will prevent us from closing self._buf during the stack
194+
# unwind.
195+
with memoryview(self._buf) as img_mv:
196+
img_data = bytearray(img_mv[:new_size])
206197

207198
return self.cls_image(img_data, monitor)
208199

0 commit comments

Comments
 (0)