MSS: better handle resources to prevent leaks

Author: BoboTiG

CHANGELOG

@@ -6,6 +6,7 @@ History:
       - removed support for Python 2.7
       - MSS: improve type annotations and add CI check
       - MSS: use __slots__ for better performances
+      - MSS: better handle resources to prevent leaks
       - Windows: use our own instances of GDI32 and User32 DLLs
       - doc: add project_urls to setup.cfg
       - doc: add an example using the multiprocessing module (closes #82)

CHANGES.rst

@@ -8,6 +8,7 @@ darwin.py
 linux.py
 --------
 - Added `MSS.__slots__`
+- Deleted `MSS.close()`
 - Deleted ``LAST_ERROR`` constant. Use ``ERROR`` namespace instead, specially the ``ERROR.details`` attribute.
 
 models.py
@@ -28,6 +29,7 @@ screenshot.py
 windows.py
 ----------
 - Added `MSS.__slots__`
+- Deleted `MSS.close()`
 
 
 4.0.1 (2019-01-26)

docs/source/api.rst

@@ -27,12 +27,6 @@ GNU/Linux
 
         GNU/Linux initializations.
 
-    .. method:: close()
-
-        Disconnect from the X server.
-
-        .. versionadded:: 4.0.0
-
     .. method:: get_error_details()
 
         :rtype: Optional[dict[str, Any]]
@@ -83,20 +77,6 @@ GNU/Linux
     .. versionadded:: 3.3.0
 
 
-Windows
--------
-
-.. module:: mss.windows
-
-.. class:: MSS
-
-    .. method:: close()
-
-        Close GDI handles and free DCs.
-
-        .. versionadded:: 4.0.0
-
-
 Methods
 =======
 
@@ -108,10 +88,7 @@ Methods
 
     .. method:: close()
 
-        Clean-up method. Does nothing by default. For more information, see specific implementations:
-
-            - :meth:`~mss.linux.MSS.close()` for GNU/Linux
-            - :meth:`~mss.windows.MSS.close()` for Windows
+        Clean-up method. Does nothing by default.
 
         .. versionadded:: 4.0.0
 

docs/source/examples/fps.py

@@ -56,7 +56,6 @@ def screen_record_efficient():
             cv2.destroyAllWindows()
             break
 
-    sct.close()
     return fps
 
 

docs/source/usage.rst

@@ -9,7 +9,6 @@ So MSS can be used as simply as::
 
     from mss import mss
 
-
 Or import the good one base on your operating system::
 
     # MacOS X
@@ -30,26 +29,6 @@ So the module can be used as simply as::
     with mss() as sct:
         # ...
 
-.. note::
-
-    On GNU/Linux and Windows, if you are using this kind of code::
-
-        sct = mss()
-        sct.shot()
-        sct.grab()
-        # or any attribute/method of sct
-
-    Then you will have to **manually** call :meth:`~mss.base.MSSMixin.close()` to free resources.
-
-.. warning::
-
-    This code is **highly** unadvised as there will be **resources leaks** without possibility to do something to clean them::
-
-        mss().shot()
-        mss().grab()
-        # or any mss().xxx or mss().xxx()
-
-
 Intensive Use
 =============
 

mss/linux.py

@@ -181,7 +181,11 @@ class MSS(MSSMixin):
     It uses intensively the Xlib and its Xrandr extension.
     """
 
-    __slots__ = {"display", "drawable", "root", "xlib", "xrandr"}
+    __slots__ = {"drawable", "root", "xlib", "xrandr"}
+
+    # Class attribute to store the display opened with XOpenDisplay().
+    # Instancied one time to prevent resource leak.
+    display = None
 
     def __init__(self, display=None):
         # type: (Optional[Union[bytes, str]]) -> None
@@ -217,8 +221,9 @@ def __init__(self, display=None):
 
         self._set_cfunctions()
 
-        self.display = self.xlib.XOpenDisplay(display)
-        self.root = self.xlib.XDefaultRootWindow(self.display)
+        if not MSS.display:
+            MSS.display = self.xlib.XOpenDisplay(display)
+        self.root = self.xlib.XDefaultRootWindow(MSS.display)
 
         # Fix for XRRGetScreenResources and XGetImage:
         #     expected LP_Display instance instead of LP_XWindowAttributes
@@ -285,7 +290,6 @@ def cfactory(attr=self.xlib, func=None, argtypes=None, restype=None):
             restype=pointer(XImage),
         )
         cfactory(func="XDestroyImage", argtypes=[pointer(XImage)], restype=void)
-        cfactory(func="XCloseDisplay", argtypes=[pointer(Display)], restype=void)
 
         # A simple benchmark calling 10 times those 2 functions:
         # XRRGetScreenResources():        0.1755971429956844 s
@@ -326,20 +330,6 @@ def cfactory(attr=self.xlib, func=None, argtypes=None, restype=None):
             restype=void,
         )
 
-    def close(self):
-        # type: () -> None
-        """
-        Disconnect from the X server to prevent:
-            Maximum number of clients reached. Segmentation fault (core dumped)
-        """
-
-        try:
-            self.xlib.XCloseDisplay(self.display)
-            # Delete the attribute to prevent interpreter crash if called twice
-            del self.display
-        except AttributeError:
-            pass
-
     def get_error_details(self):
         # type: () -> Optional[Dict[str, Any]]
         """ Get more information about the latest X server error. """
@@ -351,7 +341,7 @@ def get_error_details(self):
             ERROR.details = None
             xserver_error = ctypes.create_string_buffer(1024)
             self.xlib.XGetErrorText(
-                self.display,
+                MSS.display,
                 details.get("xerror_details", {}).get("error_code", 0),
                 xserver_error,
                 len(xserver_error),
@@ -368,9 +358,11 @@ def monitors(self):
         """ Get positions of monitors (see parent class property). """
 
         if not self._monitors:
+            display = MSS.display
+
             # All monitors
             gwa = XWindowAttributes()
-            self.xlib.XGetWindowAttributes(self.display, self.root, ctypes.byref(gwa))
+            self.xlib.XGetWindowAttributes(display, self.root, ctypes.byref(gwa))
             self._monitors.append(
                 {
                     "left": int(gwa.x),
@@ -381,11 +373,9 @@ def monitors(self):
             )
 
             # Each monitors
-            mon = self.xrandr.XRRGetScreenResourcesCurrent(self.display, self.drawable)
+            mon = self.xrandr.XRRGetScreenResourcesCurrent(display, self.drawable)
             for idx in range(mon.contents.ncrtc):
-                crtc = self.xrandr.XRRGetCrtcInfo(
-                    self.display, mon, mon.contents.crtcs[idx]
-                )
+                crtc = self.xrandr.XRRGetCrtcInfo(display, mon, mon.contents.crtcs[idx])
                 if crtc.contents.noutput == 0:
                     self.xrandr.XRRFreeCrtcInfo(crtc)
                     continue
@@ -417,7 +407,7 @@ def grab(self, monitor):
             }
 
         ximage = self.xlib.XGetImage(
-            self.display,
+            MSS.display,
             self.drawable,
             monitor["left"],
             monitor["top"],

mss/windows.py

@@ -68,28 +68,19 @@ class BITMAPINFO(ctypes.Structure):
 class MSS(MSSMixin):
     """ Multiple ScreenShots implementation for Microsoft Windows. """
 
-    __slots__ = {
-        "_bbox",
-        "_bmi",
-        "_bmp",
-        "_data",
-        "_memdc",
-        "_srcdc",
-        "gdi32",
-        "monitorenumproc",
-        "user32",
-    }
+    __slots__ = {"_bbox", "_bmi", "_data", "gdi32", "monitorenumproc", "user32"}
+
+    # Class attributes instancied one time to prevent resource leaks.
+    bmp = None
+    memdc = None
+    srcdc = None
 
     def __init__(self, **_):
         # type: (Any) -> None
         """ Windows initialisations. """
 
         super().__init__()
 
-        self._bbox = {"height": 0, "width": 0}
-        self._bmp = None
-        self._data = ctypes.create_string_buffer(0)  # type: ctypes.Array[ctypes.c_char]
-
         self.monitorenumproc = ctypes.WINFUNCTYPE(
             INT, DWORD, DWORD, ctypes.POINTER(RECT), DOUBLE
         )
@@ -99,8 +90,12 @@ def __init__(self, **_):
         self._set_cfunctions()
         self._set_dpi_awareness()
 
-        self._srcdc = self.user32.GetWindowDC(0)
-        self._memdc = self.gdi32.CreateCompatibleDC(self._srcdc)
+        self._bbox = {"height": 0, "width": 0}
+        self._data = ctypes.create_string_buffer(0)  # type: ctypes.Array[ctypes.c_char]
+
+        if not MSS.srcdc or not MSS.memdc:
+            MSS.srcdc = self.user32.GetWindowDC(0)
+            MSS.memdc = self.gdi32.CreateCompatibleDC(MSS.srcdc)
 
         bmi = BITMAPINFO()
         bmi.bmiHeader.biSize = ctypes.sizeof(BITMAPINFOHEADER)
@@ -129,17 +124,13 @@ def _set_cfunctions(self):
         self._cfactory(
             attr=self.user32, func="GetWindowDC", argtypes=[HWND], restype=HDC
         )
-        self._cfactory(
-            attr=self.user32, func="ReleaseDC", argtypes=[HWND, HGDIOBJ], restype=INT
-        )
 
         self._cfactory(
             attr=self.gdi32, func="GetDeviceCaps", argtypes=[HWND, INT], restype=INT
         )
         self._cfactory(
             attr=self.gdi32, func="CreateCompatibleDC", argtypes=[HDC], restype=HDC
         )
-        self._cfactory(attr=self.gdi32, func="DeleteDC", argtypes=[HDC], restype=BOOL)
         self._cfactory(
             attr=self.gdi32,
             func="CreateCompatibleBitmap",
@@ -168,32 +159,10 @@ def _set_cfunctions(self):
             restype=BOOL,
         )
 
-    def close(self):
-        # type: () -> None
-        """ Close GDI handles and free DCs. """
-
-        try:
-            self.gdi32.DeleteObject(self._bmp)
-            del self._bmp
-        except AttributeError:
-            pass
-
-        try:
-            self.gdi32.DeleteDC(self._memdc)
-            del self._memdc
-        except (OSError, AttributeError):
-            pass
-
-        try:
-            self.user32.ReleaseDC(0, self._srcdc)
-            del self._srcdc
-        except AttributeError:
-            pass
-
     def _set_dpi_awareness(self):
         """ Set DPI aware to capture full screen on Hi-DPI monitors. """
 
-        version = sys.getwindowsversion()[:2]
+        version = sys.getwindowsversion()[:2]  # pylint: disable=no-member
         if version >= (6, 3):
             # Windows 8.1+
             # Here 2 = PROCESS_PER_MONITOR_DPI_AWARE, which means:
@@ -288,29 +257,32 @@ def grab(self, monitor):
                 "height": monitor[3] - monitor[1],
             }
 
+        srcdc, memdc = MSS.srcdc, MSS.memdc
         width, height = monitor["width"], monitor["height"]
 
         if (self._bbox["height"], self._bbox["width"]) != (height, width):
             self._bbox = monitor
             self._bmi.bmiHeader.biWidth = width
             self._bmi.bmiHeader.biHeight = -height  # Why minus? [1]
             self._data = ctypes.create_string_buffer(width * height * 4)  # [2]
-            self._bmp = self.gdi32.CreateCompatibleBitmap(self._srcdc, width, height)
-            self.gdi32.SelectObject(self._memdc, self._bmp)
+            if MSS.bmp:
+                self.gdi32.DeleteObject(MSS.bmp)
+            MSS.bmp = self.gdi32.CreateCompatibleBitmap(srcdc, width, height)
+            self.gdi32.SelectObject(memdc, MSS.bmp)
 
         self.gdi32.BitBlt(
-            self._memdc,
+            memdc,
             0,
             0,
             width,
             height,
-            self._srcdc,
+            srcdc,
             monitor["left"],
             monitor["top"],
             SRCCOPY | CAPTUREBLT,
         )
         bits = self.gdi32.GetDIBits(
-            self._memdc, self._bmp, 0, height, self._data, self._bmi, DIB_RGB_COLORS
+            memdc, MSS.bmp, 0, height, self._data, self._bmi, DIB_RGB_COLORS
         )
         if bits != height:
             raise ScreenShotError("gdi32.GetDIBits() failed.")