Improve ext.Window memory safety

Author: a-hurst

doc/news.rst

@@ -18,6 +18,9 @@ New Features:
 * Updated to wrap new functions and constants in SDL2 2.24.0 (PR #246).
 * Added the error-handling :func:`sdl2.ext.raise_sdl_err` function to the public
   API.
+* Added informative exceptions when trying perform operations (e.g. show, hide,
+  minimize) on a closed :obj:`sdl2.ext.Window`, and generally improved memory
+  safety of the Window class.
 
 Fixed Bugs:
 
@@ -29,6 +32,8 @@ API Changes:
 
 * Moved :class:`sdl2.ext.SDLError` and :func:`sdl2.ext.raise_sdl_err`
   internally to a new submodule :mod:`sdl2.ext.err`.
+* :obj:`sdl2.ext.Window` objects can now be repositioned when closed (would
+  previously raise an ``SDLError`` exception).
 
 
 0.9.13

sdl2/ext/window.py

@@ -95,45 +95,53 @@ def __init__(self, title, size, position=None, flags=None):
         if flags is None:
             flags = self.DEFAULTFLAGS
 
+        self.window = None
         self._title = title
         self._position = position
         self._flags = flags
         self._size = size
 
-        self.window = None
         self.create()
 
     def __del__(self):
         """Releases the resources of the Window, implicitly destroying the
         underlying SDL2 window."""
         self.close()
 
+    def _ensure_window(self, action):
+        if not self.window:
+            raise RuntimeError("Cannot {0} a closed window.".format(action))
+
     @property
     def position(self):
         """tuple: The (x, y) pixel coordinates of the top-left corner of the
         window.
 
         """
+        if not self.window:
+            return self._position
         x, y = c_int(), c_int()
         video.SDL_GetWindowPosition(self.window, byref(x), byref(y))
         return x.value, y.value
 
     @position.setter
     def position(self, value):
-        if not self.window:
-            raise SDLError("The window is not available")
-        video.SDL_SetWindowPosition(self.window, value[0], value[1])
+        if self.window:
+            video.SDL_SetWindowPosition(self.window, value[0], value[1])
         self._position = value[0], value[1]
 
     @property
     def title(self):
         """str: The title of the window."""
+        if not self.window:
+            return stringify(self._title, "utf-8")
         return stringify(video.SDL_GetWindowTitle(self.window), "utf-8")
 
     @title.setter
     def title(self, value):
-        title_bytes = utf8(value).encode('utf-8')
-        video.SDL_SetWindowTitle(self.window, title_bytes)
+        if self.window:
+            title_bytes = utf8(value).encode('utf-8')
+            video.SDL_SetWindowTitle(self.window, title_bytes)
         self._title = value
 
     @property
@@ -142,30 +150,33 @@ def size(self):
         ``(width, height)``.
         
         """
+        if not self.window:
+            return self._size
         w, h = c_int(), c_int()
         video.SDL_GetWindowSize(self.window, byref(w), byref(h))
         return w.value, h.value
 
     @size.setter
     def size(self, value):
-        video.SDL_SetWindowSize(self.window, value[0], value[1])
         self._size = value[0], value[1]
+        if self.window:
+            video.SDL_SetWindowSize(self.window, value[0], value[1])
 
     def create(self):
         """Creates the window if it does not already exist."""
-        if self.window != None:
+        if self.window:
             return
         window = video.SDL_CreateWindow(utf8(self._title).encode('utf-8'),
                                         self._position[0], self._position[1],
                                         self._size[0], self._size[1],
                                         self._flags)
         if not window:
-            raise SDLError()
+            raise_sdl_err("creating the window")
         self.window = window.contents
 
     def open(self):
         """Creates and shows the window."""
-        if self.window is None:
+        if not self.window:
             self.create()
         self.show()
 
@@ -177,7 +188,7 @@ def close(self):
         :meth:`create` method.
 
         """
-        if hasattr(self, "window") and self.window != None:
+        if self.window:
             video.SDL_DestroyWindow(self.window)
             self.window = None
 
@@ -187,6 +198,7 @@ def show(self):
         If the window is already visible, this method does nothing.
 
         """
+        self._ensure_window("show")
         video.SDL_ShowWindow(self.window)
 
     def hide(self):
@@ -195,14 +207,17 @@ def hide(self):
         If the window is already hidden, this method does nothing.
         
         """
+        self._ensure_window("hide")
         video.SDL_HideWindow(self.window)
 
     def maximize(self):
         """Maximizes the window."""
+        self._ensure_window("maximize")
         video.SDL_MaximizeWindow(self.window)
 
     def minimize(self):
         """Minimizes the window into the system's dock or task bar."""
+        self._ensure_window("minimize")
         video.SDL_MinimizeWindow(self.window)
 
     def restore(self):
@@ -212,6 +227,7 @@ def restore(self):
         nothing.
 
         """
+        self._ensure_window("restore")
         video.SDL_RestoreWindow(self.window)
 
     def refresh(self):
@@ -222,6 +238,7 @@ def refresh(self):
            modified using :meth:`get_surface`.
 
         """
+        self._ensure_window("refresh")
         video.SDL_UpdateWindowSurface(self.window)
 
     def get_surface(self):
@@ -245,7 +262,8 @@ def get_surface(self):
             :obj:`~sdl2.SDL_Surface`: The surface associated with the window.
 
         """
+        self._ensure_window("get the surface of")
         sf = video.SDL_GetWindowSurface(self.window)
         if not sf:
-            raise SDLError()
+            raise_sdl_err("getting the window surface")
         return sf.contents

sdl2/test/sdl2ext_window_test.py

@@ -35,7 +35,12 @@ def test_title(self, with_sdl):
         assert window.title == "Window"
         window.title = b"Test1234"
         assert window.title == "Test1234"
+        video.SDL_SetWindowTitle(window.window, b"manual override")
+        assert window.title == "manual override"
         window.close()
+        assert window.title == "Test1234"
+        window.title = "set when closed"
+        assert window.title == "set when closed"
 
     def test_show_hide(self, with_sdl):
         get_flags = video.SDL_GetWindowFlags
@@ -46,6 +51,11 @@ def test_show_hide(self, with_sdl):
         window.hide()
         assert get_flags(window.window) & SDL_WINDOW_SHOWN != SDL_WINDOW_SHOWN
         window.close()
+        # Test informative exceptions for closed window
+        with pytest.raises(RuntimeError):
+            window.show()
+        with pytest.raises(RuntimeError):
+            window.hide()
 
     @pytest.mark.skipif(SKIP_ANNOYING, reason="Skip unless requested")
     def test_maximize(self, with_sdl):
@@ -59,6 +69,9 @@ def test_maximize(self, with_sdl):
         if not DRIVER_DUMMY:
             assert get_flags(window.window) & max_flag == max_flag
         window.close()
+        # Test informative exception for closed window
+        with pytest.raises(RuntimeError):
+            window.maximize()
 
     @pytest.mark.skipif(SKIP_ANNOYING, reason="Skip unless requested")
     def test_minimize_restore(self, with_sdl):
@@ -73,6 +86,11 @@ def test_minimize_restore(self, with_sdl):
         window.restore()
         assert get_flags(window.window) & min_flag != min_flag
         window.close()
+        # Test informative exceptions for closed window
+        with pytest.raises(RuntimeError):
+            window.minimize()
+        with pytest.raises(RuntimeError):
+            window.restore()
 
     @pytest.mark.skip("not implemented")
     def test_refresh(self, with_sdl):
@@ -83,6 +101,9 @@ def test_get_surface(self, with_sdl):
         sf = window.get_surface()
         assert isinstance(sf, surface.SDL_Surface)
         window.close()
+        # Test informative exception for closed window
+        with pytest.raises(RuntimeError):
+            sf = window.get_surface()
 
     def test_open_close(self, with_sdl):
         get_flags = video.SDL_GetWindowFlags