Issue #17435: Don't use mutable default values in Timer.

Patch by Denver Coneybeare with some test modifications by me.

diff --git a/Doc/library/threading.rst b/Doc/library/threading.rst
index f697cbb4ded041e7c6e7a10f47cbd95c6fe4c403..1bcf0a2ea51e4a0b915415802d481cd15224143d 100644 (file)
--- a/Doc/library/threading.rst
+++ b/Doc/library/threading.rst
@@ -839,10 +839,12 @@ For example::
    t.start() # after 30 seconds, "hello, world" will be printed
 
 
-.. class:: Timer(interval, function, args=[], kwargs={})
+.. class:: Timer(interval, function, args=None, kwargs=None)
 
    Create a timer that will run *function* with arguments *args* and  keyword
    arguments *kwargs*, after *interval* seconds have passed.
+   If *args* is None (the default) then an empty list will be used.
+   If *kwargs* is None (the default) then an empty dict will be used.
 
    .. versionchanged:: 3.3
       changed from a factory function to a class.

diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
index 11e63d3e8304af4269f5b5ebc7fc56f46c66c30f..e6b209da2a15812b5d352ed0f4ff57c33ee1f8f6 100644 (file)
--- a/Lib/test/test_threading.py
+++ b/Lib/test/test_threading.py
@@ -786,6 +786,32 @@ class ThreadingExceptionTests(BaseTestCase):
         self.assertEqual(p.returncode, 0, "Unexpected error: " + stderr.decode())
         self.assertEqual(data, expected_output)
 
+class TimerTests(BaseTestCase):
+
+    def setUp(self):
+        BaseTestCase.setUp(self)
+        self.callback_args = []
+        self.callback_event = threading.Event()
+
+    def test_init_immutable_default_args(self):
+        # Issue 17435: constructor defaults were mutable objects, they could be
+        # mutated via the object attributes and affect other Timer objects.
+        timer1 = threading.Timer(0.01, self._callback_spy)
+        timer1.start()
+        self.callback_event.wait()
+        timer1.args.append("blah")
+        timer1.kwargs["foo"] = "bar"
+        self.callback_event.clear()
+        timer2 = threading.Timer(0.01, self._callback_spy)
+        timer2.start()
+        self.callback_event.wait()
+        self.assertEqual(len(self.callback_args), 2)
+        self.assertEqual(self.callback_args, [((), {}), ((), {})])
+
+    def _callback_spy(self, *args, **kwargs):
+        self.callback_args.append((args[:], kwargs.copy()))
+        self.callback_event.set()
+
 class LockTests(lock_tests.LockTests):
     locktype = staticmethod(threading.Lock)
 
@@ -815,16 +841,5 @@ class BoundedSemaphoreTests(lock_tests.BoundedSemaphoreTests):
 class BarrierTests(lock_tests.BarrierTests):
     barriertype = staticmethod(threading.Barrier)
 
-
-def test_main():
-    test.support.run_unittest(LockTests, PyRLockTests, CRLockTests, EventTests,
-                              ConditionAsRLockTests, ConditionTests,
-                              SemaphoreTests, BoundedSemaphoreTests,
-                              ThreadTests,
-                              ThreadJoinOnShutdown,
-                              ThreadingExceptionTests,
-                              BarrierTests,
-                              )
-
 if __name__ == "__main__":
-    test_main()
+    unittest.main()

diff --git a/Lib/threading.py b/Lib/threading.py
index 6c34d4978291fa64247861b2c362911f1d04ca2e..46df676f24181dd8249a4dc736f84473039e2499 100644 (file)
--- a/Lib/threading.py
+++ b/Lib/threading.py
@@ -802,17 +802,17 @@ class Thread:
 class Timer(Thread):
     """Call a function after a specified number of seconds:
 
-    t = Timer(30.0, f, args=[], kwargs={})
+    t = Timer(30.0, f, args=None, kwargs=None)
     t.start()
     t.cancel() # stop the timer's action if it's still waiting
     """
 
-    def __init__(self, interval, function, args=[], kwargs={}):
+    def __init__(self, interval, function, args=None, kwargs=None):
         Thread.__init__(self)
         self.interval = interval
         self.function = function
-        self.args = args
-        self.kwargs = kwargs
+        self.args = args if args is not None else []
+        self.kwargs = kwargs if kwargs is not None else {}
         self.finished = Event()
 
     def cancel(self):

diff --git a/Misc/NEWS b/Misc/NEWS
index c3e9ef2fa747358848184dd932eb212f55a79f46..beb3a8df392c6ec685f2f930ac1d6eb15cc8ffba 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -15,6 +15,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #17435: threading.Timer's __init__ method no longer uses mutable
+  default values for the args and kwargs parameters.
+
 - Issue #17526: fix an IndexError raised while passing code without filename to
   inspect.findsource().  Initial patch by Tyler Doyle.