Fill the clipboard buffer before emptying the clipboard

EmptyClipboard is the point of no return: after it, whatever was there is gone,
and the allocation that failed on the next line left the clipboard holding
nothing. That is the one path where restoring what a dictation borrowed could
lose it instead. The buffer is filled first, and the clipboard is opened only
once there is something to put in it.

paste.py

@@ -412,19 +412,26 @@ def _win_read_text():
 def _win_write_text(text):
     user32, kernel32 = _win_api()
     payload = str(text).encode("utf-16-le") + b"\x00\x00"
-    if not _win_open_clipboard(user32):
-        raise PasteError(t("Could not copy to clipboard: {error}",
-                           error="the clipboard is held by another program"))
-    handle = None
-    try:
-        user32.EmptyClipboard()
+    # Filled before the clipboard is opened at all. EmptyClipboard is what
+    # throws away whatever was there, and a failure after it and before the
+    # SetClipboardData would leave the clipboard holding nothing: the one way
+    # this function could lose what it was called to put back.
     handle = kernel32.GlobalAlloc(_WIN_GMEM_MOVEABLE, len(payload))
     pointer = kernel32.GlobalLock(handle) if handle else None
     if not pointer:
+        if handle:
+            kernel32.GlobalFree(handle)
         raise PasteError(t("Could not copy to clipboard: {error}",
                            error="out of memory"))
     ctypes.memmove(pointer, payload, len(payload))
     kernel32.GlobalUnlock(handle)
+
+    if not _win_open_clipboard(user32):
+        kernel32.GlobalFree(handle)
+        raise PasteError(t("Could not copy to clipboard: {error}",
+                           error="the clipboard is held by another program"))
+    try:
+        user32.EmptyClipboard()
         if not user32.SetClipboardData(_WIN_CF_UNICODETEXT, handle):
             raise PasteError(t("Could not copy to clipboard: {error}",
                                error=f"error {_win_error()}"))

tests/test_paste.py

@@ -484,6 +484,8 @@ class FakeWin32:
         self.next_handle = 1
         self.pressed = []          # (virtual key, flags), in the order sent
         self.send_result = None    # None: report every event as delivered
+        self.held = False          # another program has the clipboard open
+        self.out_of_memory = False
 
     def _keep(self, buffer):
         handle = self.next_handle
@@ -493,7 +495,7 @@ class FakeWin32:
 
     # --- user32
     def OpenClipboard(self, owner):
-        return 1
+        return 0 if self.held else 1
 
     def CloseClipboard(self):
         return 1
@@ -519,6 +521,8 @@ class FakeWin32:
 
     # --- kernel32
     def GlobalAlloc(self, flags, size):
+        if self.out_of_memory:
+            return 0
         return self._keep(ctypes.create_string_buffer(size))
 
     def GlobalLock(self, handle):
@@ -559,6 +563,23 @@ class Windows(Standing, DikteTest):
         paste.copy_bytes(saved)
         self.assertEqual(self.api.text, "mine")
 
+    def test_a_copy_that_fails_leaves_what_was_there(self):
+        """EmptyClipboard is the point of no return, so nothing runs after it."""
+        paste.copy("mine")
+        for failure in ("out_of_memory", "held"):
+            with self.subTest(failure=failure):
+                setattr(self.api, failure, True)
+                with self.assertRaises(paste.PasteError):
+                    paste.copy("the dictation")
+                self.assertEqual(self.api.text, "mine")
+                setattr(self.api, failure, False)
+
+    def test_the_handle_is_not_leaked_when_the_copy_fails(self):
+        self.api.held = True
+        with self.assertRaises(paste.PasteError):
+            paste.copy("the dictation")
+        self.assertEqual(self.api.buffers, {})
+
     def test_readiness_asks_for_no_program_and_no_permission(self):
         with only_these_tools():
             self.assertTrue(paste.paste_ready())