Merge pull request #1762 from minrk/destroy-del

changelog for 24

Author: minrk

docs/source/changelog.md

@@ -5,6 +5,28 @@
 This is a coarse summary of changes in pyzmq versions.
 For a full changelog, consult the [git log](https://github.com/zeromq/pyzmq/commits).
 
+## 24
+
+pyzmq 24 has two breaking changes (one only on Windows), though they are not likely to affect most users.
+
+Breaking changes:
+
+- Due to a libzmq bug causing unavoidable crashes for some users,
+  Windows wheels no longer bundle libzmq with AF_UNIX support.
+  In order to enable AF_UNIX on Windows, pyzmq must be built from source,
+  linking an appropriate build of libzmq (e.g. `libzmq-v142`).
+  AF_UNIX support will be re-enabled in pyzmq wheels
+  when libzmq published fixed releases.
+
+- Using a {class}`zmq.Context` as a context manager or deleting a context without closing it now calls {meth}`zmq.Context.destroy` at exit instead of {meth}`zmq.Context.term`.
+  This will have little effect on most users,
+  but changes what happens when user bugs result in a context being _implicitly_ destroyed while sockets are left open.
+  In almost all cases, this will turn what used to be a hang into a warning.
+  However, there may be some cases where sockets are actively used in threads,
+  which could result in a crash.
+  To use sockets across threads, it is critical to properly and explicitly close your contexts and sockets,
+  which will always avoid this issue.
+
 ## 23.2.1
 
 Improvements:

zmq/sugar/context.py

@@ -40,18 +40,23 @@ class Context(ContextBase, AttributeSetter, Generic[ST]):
     .. versionchanged:: 24
 
         When using a Context as a context manager (``with zmq.Context()``),
-        exiting the context calls `ctx.destroy()`,
+        or deleting a context without closing it first,
+        ``ctx.destroy()`` is called,
         closing any leftover sockets,
         instead of `ctx.term()` which requires sockets to be closed first.
-        This makes contexts-as-context-managers
-        not safe for use with sockets managed in other threads.
+
+        This prevents hangs caused by `ctx.term()` if sockets are left open,
+        but means that unclean destruction of contexts
+        (with sockets left open) is not safe
+        if sockets are managed in other threads.
     """
 
     sockopts: Dict[int, Any]
     _instance: Any = None
     _instance_lock = Lock()
     _instance_pid: Optional[int] = None
     _shadow = False
+    _warn_destroy_close = False
     _sockets: WeakSet
     # mypy doesn't like a default value here
     _socket_class: Type[ST] = Socket  # type: ignore
@@ -66,19 +71,26 @@ def __init__(self: "Context[Socket]", io_threads: int = 1, **kwargs: Any) -> Non
         self._sockets = WeakSet()
 
     def __del__(self) -> None:
-        """deleting a Context should terminate it, without trying non-threadsafe destroy"""
+        """Deleting a Context without closing it destroys it and all sockets.
+
+        .. versionchanged:: 24
+            Switch from threadsafe `term()` which hangs in the event of open sockets
+            to less safe `destroy()` which
+            warns about any leftover sockets and closes them.
+        """
 
         # Calling locals() here conceals issue #1167 on Windows CPython 3.5.4.
         locals()
 
         if not self._shadow and not _exiting and not self.closed:
+            self._warn_destroy_close = True
             warnings.warn(
                 f"unclosed context {self}",
                 ResourceWarning,
                 stacklevel=2,
                 source=self,
             )
-            self.term()
+            self.destroy()
 
     _repr_cls = "zmq.Context"
 
@@ -102,15 +114,8 @@ def __enter__(self: T) -> T:
         return self
 
     def __exit__(self, exc_type: Any, exc_value: Any, traceback: Any) -> None:
-        for socket in list(self._sockets):
-            if socket and not socket.closed:
-                warnings.warn(
-                    f"unclosed socket {socket}",
-                    ResourceWarning,
-                    stacklevel=2,
-                    source=socket,
-                )
-
+        # warn about any leftover sockets before closing them
+        self._warn_destroy_close = True
         self.destroy()
 
     def __copy__(self: T, memo: Any = None) -> T:
@@ -254,6 +259,13 @@ def destroy(self, linger: Optional[float] = None) -> None:
         self._sockets = WeakSet()
         for s in sockets:
             if s and not s.closed:
+                if self._warn_destroy_close:
+                    warnings.warn(
+                        f"Destroying context with unclosed socket {s}",
+                        ResourceWarning,
+                        stacklevel=3,
+                        source=s,
+                    )
                 if linger is not None:
                     s.setsockopt(SocketOption.LINGER, linger)
                 s.close()