[3.8] bpo-37322: Fix ResourceWarning and exception handling in test (GH-25553)

Revert 73ea546, increase logging, and improve stability of test.

Handle all OSErrors in a single block. OSError also takes care of
SSLError and socket's connection errors.

Partly reverts commit fb7e750. The
threaded connection handler must not raise an unhandled exception..
(cherry picked from commit c8666cf)

Co-authored-by: Christian Heimes <[email protected]>

Author: tiran

.github/workflows/build.yml

@@ -184,7 +184,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        openssl_ver: [1.0.2u, 1.1.0l, 1.1.1k, 3.0.0-alpha14]
+        openssl_ver: [1.0.2u, 1.1.0l, 1.1.1k, 3.0.0-alpha15]
     env:
       OPENSSL_VER: ${{ matrix.openssl_ver }}
       MULTISSL_DIR: ${{ github.workspace }}/multissl

Lib/test/test_ssl.py

@@ -2392,7 +2392,10 @@ def wrap_conn(self):
                         sys.stdout.write(" client cert is " + pprint.pformat(cert) + "\n")
                     cert_binary = self.sslconn.getpeercert(True)
                     if support.verbose and self.server.chatty:
-                        sys.stdout.write(" cert binary is " + str(len(cert_binary)) + " bytes\n")
+                        if cert_binary is None:
+                            sys.stdout.write(" client did not provide a cert\n")
+                        else:
+                            sys.stdout.write(f" cert binary is {len(cert_binary)}b\n")
                 cipher = self.sslconn.cipher()
                 if support.verbose and self.server.chatty:
                     sys.stdout.write(" server: connection cipher is now " + str(cipher) + "\n")
@@ -2490,31 +2493,22 @@ def run(self):
                             sys.stdout.write(" server: read %r (%s), sending back %r (%s)...\n"
                                              % (msg, ctype, msg.lower(), ctype))
                         self.write(msg.lower())
-                except (ConnectionResetError, ConnectionAbortedError):
-                    # XXX: OpenSSL 1.1.1 sometimes raises ConnectionResetError
-                    # when connection is not shut down gracefully.
+                except OSError as e:
+                    # handles SSLError and socket errors
                     if self.server.chatty and support.verbose:
-                        sys.stdout.write(
-                            " Connection reset by peer: {}\n".format(
-                                self.addr)
-                        )
-                    self.close()
-                    self.running = False
-                except ssl.SSLError as err:
-                    # On Windows sometimes test_pha_required_nocert receives the
-                    # PEER_DID_NOT_RETURN_A_CERTIFICATE exception
-                    # before the 'tlsv13 alert certificate required' exception.
-                    # If the server is stopped when PEER_DID_NOT_RETURN_A_CERTIFICATE
-                    # is received test_pha_required_nocert fails with ConnectionResetError
-                    # because the underlying socket is closed
-                    if 'PEER_DID_NOT_RETURN_A_CERTIFICATE' == err.reason:
-                        if self.server.chatty and support.verbose:
-                            sys.stdout.write(err.args[1])
-                        # test_pha_required_nocert is expecting this exception
-                        raise ssl.SSLError('tlsv13 alert certificate required')
-                except OSError:
-                    if self.server.chatty:
-                        handle_error("Test server failure:\n")
+                        if isinstance(e, ConnectionError):
+                            # OpenSSL 1.1.1 sometimes raises
+                            # ConnectionResetError when connection is not
+                            # shut down gracefully.
+                            print(
+                                f" Connection reset by peer: {self.addr}"
+                            )
+                        else:
+                            handle_error("Test server failure:\n")
+                    try:
+                        self.write(b"ERROR\n")
+                    except OSError:
+                        pass
                     self.close()
                     self.running = False
 
@@ -4496,24 +4490,30 @@ def test_pha_required_nocert(self):
         server_context.verify_mode = ssl.CERT_REQUIRED
         client_context.post_handshake_auth = True
 
-        # Ignore expected SSLError in ConnectionHandler of ThreadedEchoServer
-        # (it is only raised sometimes on Windows)
-        with support.catch_threading_exception() as cm:
-            server = ThreadedEchoServer(context=server_context, chatty=False)
-            with server:
-                with client_context.wrap_socket(socket.socket(),
-                                                server_hostname=hostname) as s:
-                    s.connect((HOST, server.port))
-                    s.write(b'PHA')
+        def msg_cb(conn, direction, version, content_type, msg_type, data):
+            if support.verbose and content_type == _TLSContentType.ALERT:
+                info = (conn, direction, version, content_type, msg_type, data)
+                sys.stdout.write(f"TLS: {info!r}\n")
+
+        server_context._msg_callback = msg_cb
+        client_context._msg_callback = msg_cb
+
+        server = ThreadedEchoServer(context=server_context, chatty=True)
+        with server:
+            with client_context.wrap_socket(socket.socket(),
+                                            server_hostname=hostname) as s:
+                s.connect((HOST, server.port))
+                s.write(b'PHA')
+                with self.assertRaisesRegex(
+                    ssl.SSLError,
+                    'tlsv13 alert certificate required'
+                ):
                     # receive CertificateRequest
                     self.assertEqual(s.recv(1024), b'OK\n')
                     # send empty Certificate + Finish
                     s.write(b'HASCERT')
                     # receive alert
-                    with self.assertRaisesRegex(
-                            ssl.SSLError,
-                            'tlsv13 alert certificate required'):
-                        s.recv(1024)
+                    s.recv(1024)
 
     def test_pha_optional(self):
         if support.verbose:

Tools/ssl/multissltests.py

@@ -50,7 +50,7 @@
 
 OPENSSL_RECENT_VERSIONS = [
     "1.1.1k",
-    # "3.0.0-alpha14"
+    # "3.0.0-alpha15"
 ]
 
 LIBRESSL_OLD_VERSIONS = [