Cleanups for some minor potential future issues

I asked GitHub Copilot to review my changes.  It didn't see any
problems, but I looked at its thought process and cleaned up things
that it had to think carefully about.

* Fix typo in an older CHANGELOG entry.
* Add comments about why each documented public attribute of MSSBase,
  and a few undocumented but not _-prefixed attributes of MSS
  implementations, are safe to not be protected under the lock.
* Document that the user isn't supposed to change with_cursor.  Also
  document that MSS might ignore it.
* Clarify that the comment "The attributes below are protected by
  self._lock" is meant to contrast with the attributes above that
  comment.
* In MSSBase.monitors, read the return value self._monitors under the
  lock.  Current code doesn't ever change self._monitors once it's
  created, so it's safe to return outside the lock, but verifying that
  requires inspection of the rest of the codebase.  This makes it clear
  and future-proof.
* Add a test that we can use the same MSS object on multiple threads.

Author: jholveck

CHANGELOG.md

@@ -20,7 +20,7 @@ See Git checking messages for full history.
 ## 10.0.0 (2024-11-14)
 - removed support for Python 3.8
 - added support for Python 3.14
-- Linux: fixed a threadding issue in `.close()` when calling `XCloseDisplay()` (#251)
+- Linux: fixed a threading issue in `.close()` when calling `XCloseDisplay()` (#251)
 - Linux: minor optimization when checking for a X extension status (#251)
 - :heart: contributors: @kianmeng, @shravanasati, @mgorny
 

src/mss/base.py

@@ -73,17 +73,29 @@ def __init__(
         # Mac only
         max_displays: int = 32,  # noqa: ARG002
     ) -> None:
+        # The cls_image is only used atomically, so does not require locking.
         self.cls_image: type[ScreenShot] = ScreenShot
+        # The compression level is only used atomically, so does not require locking.
         #: PNG compression level used when saving the screenshot data into a file
         #: (see :py:func:`zlib.compress()` for details).
         #:
         #: .. versionadded:: 3.2.0
         self.compression_level = compression_level
+        # The with_cursor attribute is not meant to be changed after initialization.
         #: Include the mouse cursor in screenshots.
         #:
+        #: In some circumstances, it may not be possible to include the cursor.  In that case, MSS will automatically
+        #: change this to False when the object is created.
+        #:
+        #: This should not be changed after creating the object.
+        #:
         #: .. versionadded:: 8.0.0
         self.with_cursor = with_cursor
-        # All attributes below are protected by self._lock.
+
+        # The attributes below are protected by self._lock.  The attributes above are user-visible, so we don't
+        # control when they're modified.  Currently, we only make sure that they're safe to modify while locked, or
+        # document that the user shouldn't change them.  We could also use properties protect them against changes, or
+        # change them under the lock.
         self._lock = Lock()
         self._monitors: Monitors = []
         self._closed = False
@@ -209,8 +221,7 @@ def monitors(self) -> Monitors:
         with self._lock:
             if not self._monitors:
                 self._monitors_impl()
-
-        return self._monitors
+            return self._monitors
 
     def save(
         self,

src/mss/darwin.py

@@ -120,6 +120,7 @@ class MSS(MSSBase):
     def __init__(self, /, **kwargs: Any) -> None:
         super().__init__(**kwargs)
 
+        # max_displays is only used by _monitors_impl, while the lock is held.
         #: Maximum number of displays to handle.
         self.max_displays = kwargs.get("max_displays", 32)
 

src/mss/windows.py

@@ -139,6 +139,7 @@ class MSS(MSSBase):
     def __init__(self, /, **kwargs: Any) -> None:
         super().__init__(**kwargs)
 
+        # user32 and gdi32 should not be changed after initialization.
         self.user32 = ctypes.WinDLL("user32", use_last_error=True)
         self.gdi32 = ctypes.WinDLL("gdi32", use_last_error=True)
         self._set_cfunctions()

src/tests/test_implementation.py

@@ -24,6 +24,7 @@
 
 if TYPE_CHECKING:  # pragma: nocover
     from collections.abc import Callable
+    from typing import Any
 
     from mss.models import Monitor
 
@@ -299,27 +300,45 @@ def test_grab_with_tuple_percents(mss_impl: Callable[..., MSSBase]) -> None:
         assert im.rgb == im2.rgb
 
 
-def test_thread_safety(backend: str) -> None:
-    """Regression test for issue #169."""
+class TestThreadSafety:
+    def run_test(self, do_grab: Callable[[], Any]) -> None:
+        def record() -> None:
+            """Record for one second."""
+            start_time = time.time()
+            while time.time() - start_time < 1:
+                do_grab()
 
-    def record(check: dict) -> None:
-        """Record for one second."""
-        start_time = time.time()
-        while time.time() - start_time < 1:
-            with mss.mss(backend=backend) as sct:
-                sct.grab(sct.monitors[1])
+            checkpoint[threading.current_thread()] = True
+
+        checkpoint: dict = {}
+        t1 = threading.Thread(target=record)
+        t2 = threading.Thread(target=record)
+
+        t1.start()
+        time.sleep(0.5)
+        t2.start()
 
-        check[threading.current_thread()] = True
+        t1.join()
+        t2.join()
 
-    checkpoint: dict = {}
-    t1 = threading.Thread(target=record, args=(checkpoint,))
-    t2 = threading.Thread(target=record, args=(checkpoint,))
+        assert len(checkpoint) == 2
+
+    def test_issue_169(self, backend: str) -> None:
+        """Regression test for issue #169."""
+
+        def do_grab() -> None:
+            with mss.mss(backend=backend) as sct:
+                sct.grab(sct.monitors[1])
 
-    t1.start()
-    time.sleep(0.5)
-    t2.start()
+        self.run_test(do_grab)
 
-    t1.join()
-    t2.join()
+    def test_same_object_multiple_threads(self, backend: str) -> None:
+        """Ensure that the same MSS object can be used by multiple threads.
 
-    assert len(checkpoint) == 2
+        This also implicitly tests that it can be used on a thread
+        different than the one that created it.
+        """
+        if backend == "xlib":
+            pytest.skip("The xlib backend does not support this ability")
+        with mss.mss(backend=backend) as sct:
+            self.run_test(lambda: sct.grab(sct.monitors[1]))