bpo-34246: Make sure test_smtplib always cleans resources when finished (GH-9108)

* Make sure that when some of the tests in test_smtplib fail, the allocated threads
and sockets are not leaked. 

* Use support.join_thread() instead of thread.join() to avoid infinite blocks.

1 files changed, 47 insertions, 7 deletions

diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py
index 8a29e98..0c863ed 100644
--- a/Lib/test/test_smtplib.py
+++ b/Lib/test/test_smtplib.py
@@ -20,6 +20,7 @@ import threading
 import unittest
 from test import support, mock_socket
 from test.support import HOST, HOSTv4, HOSTv6
+from test.support import threading_setup, threading_cleanup, join_thread
 from unittest.mock import Mock
 
 
@@ -193,6 +194,7 @@ class DebuggingServerTests(unittest.TestCase):
     maxDiff = None
 
     def setUp(self):
+        self.thread_key = threading_setup()
         self.real_getfqdn = socket.getfqdn
         socket.getfqdn = mock_socket.getfqdn
         # temporarily replace sys.stdout to capture DebuggingServer output
@@ -224,12 +226,15 @@ class DebuggingServerTests(unittest.TestCase):
         self.client_evt.set()
         # wait for the server thread to terminate
         self.serv_evt.wait()
-        self.thread.join()
+        join_thread(self.thread)
         # restore sys.stdout
         sys.stdout = self.old_stdout
         # restore DEBUGSTREAM
         smtpd.DEBUGSTREAM.close()
         smtpd.DEBUGSTREAM = self.old_DEBUGSTREAM
+        del self.thread
+        self.doCleanups()
+        threading_cleanup(*self.thread_key)
 
     def get_output_without_xpeer(self):
         test_output = self.output.getvalue()
@@ -247,6 +252,7 @@ class DebuggingServerTests(unittest.TestCase):
         try:
             smtp = smtplib.SMTP(self.host, self.port, local_hostname='localhost',
                                 timeout=3, source_address=(self.host, src_port))
+            self.addCleanup(smtp.close)
             self.assertEqual(smtp.source_address, (self.host, src_port))
             self.assertEqual(smtp.local_hostname, 'localhost')
             smtp.quit()
@@ -257,12 +263,14 @@ class DebuggingServerTests(unittest.TestCase):
 
     def testNOOP(self):
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         expected = (250, b'OK')
         self.assertEqual(smtp.noop(), expected)
         smtp.quit()
 
     def testRSET(self):
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         expected = (250, b'OK')
         self.assertEqual(smtp.rset(), expected)
         smtp.quit()
@@ -270,6 +278,7 @@ class DebuggingServerTests(unittest.TestCase):
     def testELHO(self):
         # EHLO isn't implemented in DebuggingServer
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         expected = (250, b'\nSIZE 33554432\nHELP')
         self.assertEqual(smtp.ehlo(), expected)
         smtp.quit()
@@ -277,6 +286,7 @@ class DebuggingServerTests(unittest.TestCase):
     def testEXPNNotImplemented(self):
         # EXPN isn't implemented in DebuggingServer
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         expected = (502, b'EXPN not implemented')
         smtp.putcmd('EXPN')
         self.assertEqual(smtp.getreply(), expected)
@@ -284,6 +294,7 @@ class DebuggingServerTests(unittest.TestCase):
 
     def testVRFY(self):
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         expected = (252, b'Cannot VRFY user, but will accept message ' + \
                          b'and attempt delivery')
         self.assertEqual(smtp.vrfy('nobody@nowhere.com'), expected)
@@ -294,6 +305,7 @@ class DebuggingServerTests(unittest.TestCase):
         # check that a second HELO returns a message that it's a duplicate
         # (this behavior is specific to smtpd.SMTPChannel)
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.helo()
         expected = (503, b'Duplicate HELO/EHLO')
         self.assertEqual(smtp.helo(), expected)
@@ -301,6 +313,7 @@ class DebuggingServerTests(unittest.TestCase):
 
     def testHELP(self):
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         self.assertEqual(smtp.help(), b'Supported commands: EHLO HELO MAIL ' + \
                                       b'RCPT DATA RSET NOOP QUIT VRFY')
         smtp.quit()
@@ -309,6 +322,7 @@ class DebuggingServerTests(unittest.TestCase):
         # connect and send mail
         m = 'A test message'
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.sendmail('John', 'Sally', m)
         # XXX(nnorwitz): this test is flaky and dies with a bad file descriptor
         # in asyncore.  This sleep might help, but should really be fixed
@@ -325,6 +339,7 @@ class DebuggingServerTests(unittest.TestCase):
     def testSendBinary(self):
         m = b'A test message'
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.sendmail('John', 'Sally', m)
         # XXX (see comment in testSend)
         time.sleep(0.01)
@@ -340,6 +355,7 @@ class DebuggingServerTests(unittest.TestCase):
         # Issue 12283
         m = '.A test\n.mes.sage.'
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.sendmail('John', 'Sally', m)
         # XXX (see comment in testSend)
         time.sleep(0.01)
@@ -354,6 +370,7 @@ class DebuggingServerTests(unittest.TestCase):
     def testSendNullSender(self):
         m = 'A test message'
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.sendmail('<>', 'Sally', m)
         # XXX (see comment in testSend)
         time.sleep(0.01)
@@ -371,6 +388,7 @@ class DebuggingServerTests(unittest.TestCase):
     def testSendMessage(self):
         m = email.mime.text.MIMEText('A test message')
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.send_message(m, from_addr='John', to_addrs='Sally')
         # XXX (see comment in testSend)
         time.sleep(0.01)
@@ -395,6 +413,7 @@ class DebuggingServerTests(unittest.TestCase):
         m['CC'] = 'Sally, Fred'
         m['Bcc'] = 'John Root <root@localhost>, "Dinsdale" <warped@silly.walks.com>'
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.send_message(m)
         # XXX (see comment in testSend)
         time.sleep(0.01)
@@ -428,6 +447,7 @@ class DebuggingServerTests(unittest.TestCase):
         m['From'] = 'foo@bar.com'
         m['To'] = 'John, Dinsdale'
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.send_message(m)
         # XXX (see comment in testSend)
         time.sleep(0.01)
@@ -455,6 +475,7 @@ class DebuggingServerTests(unittest.TestCase):
         m['From'] = 'foo@bar.com'
         m['To'] = 'John, Dinsdale'
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.send_message(m, from_addr='joe@example.com', to_addrs='foo@example.net')
         # XXX (see comment in testSend)
         time.sleep(0.01)
@@ -485,6 +506,7 @@ class DebuggingServerTests(unittest.TestCase):
         m['Sender'] = 'the_rescuers@Rescue-Aid-Society.com'
         m['To'] = 'John, Dinsdale'
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.send_message(m)
         # XXX (see comment in testSend)
         time.sleep(0.01)
@@ -517,6 +539,7 @@ class DebuggingServerTests(unittest.TestCase):
         m['Resent-To'] = 'Martha <my_mom@great.cooker.com>, Jeff'
         m['Resent-Bcc'] = 'doe@losthope.net'
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         smtp.send_message(m)
         # XXX (see comment in testSend)
         time.sleep(0.01)
@@ -555,6 +578,7 @@ class DebuggingServerTests(unittest.TestCase):
         m['Resent-To'] = 'holy@grail.net'
         m['Resent-From'] = 'Martha <my_mom@great.cooker.com>, Jeff'
         smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=3)
+        self.addCleanup(smtp.close)
         with self.assertRaises(ValueError):
             smtp.send_message(m)
         smtp.close()
@@ -630,6 +654,7 @@ class TooLongLineTests(unittest.TestCase):
     respdata = b'250 OK' + (b'.' * smtplib._MAXLINE * 2) + b'\n'
 
     def setUp(self):
+        self.thread_key = threading_setup()
         self.old_stdout = sys.stdout
         self.output = io.StringIO()
         sys.stdout = self.output
@@ -639,15 +664,18 @@ class TooLongLineTests(unittest.TestCase):
         self.sock.settimeout(15)
         self.port = support.bind_port(self.sock)
         servargs = (self.evt, self.respdata, self.sock)
-        thread = threading.Thread(target=server, args=servargs)
-        thread.start()
-        self.addCleanup(thread.join)
+        self.thread = threading.Thread(target=server, args=servargs)
+        self.thread.start()
         self.evt.wait()
         self.evt.clear()
 
     def tearDown(self):
         self.evt.wait()
         sys.stdout = self.old_stdout
+        join_thread(self.thread)
+        del self.thread
+        self.doCleanups()
+        threading_cleanup(*self.thread_key)
 
     def testLineTooLong(self):
         self.assertRaises(smtplib.SMTPResponseException, smtplib.SMTP,
@@ -877,6 +905,7 @@ class SimSMTPServer(smtpd.SMTPServer):
 class SMTPSimTests(unittest.TestCase):
 
     def setUp(self):
+        self.thread_key = threading_setup()
         self.real_getfqdn = socket.getfqdn
         socket.getfqdn = mock_socket.getfqdn
         self.serv_evt = threading.Event()
@@ -899,7 +928,10 @@ class SMTPSimTests(unittest.TestCase):
         self.client_evt.set()
         # wait for the server thread to terminate
         self.serv_evt.wait()
-        self.thread.join()
+        join_thread(self.thread)
+        del self.thread
+        self.doCleanups()
+        threading_cleanup(*self.thread_key)
 
     def testBasic(self):
         # smoke test
@@ -1162,6 +1194,7 @@ class SMTPUTF8SimTests(unittest.TestCase):
     maxDiff = None
 
     def setUp(self):
+        self.thread_key = threading_setup()
         self.real_getfqdn = socket.getfqdn
         socket.getfqdn = mock_socket.getfqdn
         self.serv_evt = threading.Event()
@@ -1186,7 +1219,10 @@ class SMTPUTF8SimTests(unittest.TestCase):
         self.client_evt.set()
         # wait for the server thread to terminate
         self.serv_evt.wait()
-        self.thread.join()
+        join_thread(self.thread)
+        del self.thread
+        self.doCleanups()
+        threading_cleanup(*self.thread_key)
 
     def test_test_server_supports_extensions(self):
         smtp = smtplib.SMTP(
@@ -1283,6 +1319,7 @@ class SimSMTPAUTHInitialResponseServer(SimSMTPServer):
 
 class SMTPAUTHInitialResponseSimTests(unittest.TestCase):
     def setUp(self):
+        self.thread_key = threading_setup()
         self.real_getfqdn = socket.getfqdn
         socket.getfqdn = mock_socket.getfqdn
         self.serv_evt = threading.Event()
@@ -1306,7 +1343,10 @@ class SMTPAUTHInitialResponseSimTests(unittest.TestCase):
         self.client_evt.set()
         # wait for the server thread to terminate
         self.serv_evt.wait()
-        self.thread.join()
+        join_thread(self.thread)
+        del self.thread
+        self.doCleanups()
+        threading_cleanup(*self.thread_key)
 
     def testAUTH_PLAIN_initial_response_login(self):
         self.serv.add_feature('AUTH PLAIN')