bpo-33238: Add InvalidStateError to concurrent.futures. (GH-7056)

Future.set_result and Future.set_exception now raise InvalidStateError
if the futures are not pending or running. This mirrors the behavior
of asyncio.Future, and prevents AssertionErrors in asyncio.wrap_future
when set_result is called multiple times.

diff --git a/Doc/library/concurrent.futures.rst b/Doc/library/concurrent.futures.rst
index 707d24dc2529ccc6def6a704ffb3288fcc80fafa..6934acc7f88e279d5ea79a8405d4529efce9b95d 100644 (file)
--- a/Doc/library/concurrent.futures.rst
+++ b/Doc/library/concurrent.futures.rst
@@ -380,6 +380,11 @@ The :class:`Future` class encapsulates the asynchronous execution of a callable.
        This method should only be used by :class:`Executor` implementations and
        unit tests.
 
+       .. versionchanged:: 3.8
+          This method raises
+          :exc:`concurrent.futures.InvalidStateError` if the :class:`Future` is
+          already done.
+
     .. method:: set_exception(exception)
 
        Sets the result of the work associated with the :class:`Future` to the
@@ -388,6 +393,10 @@ The :class:`Future` class encapsulates the asynchronous execution of a callable.
        This method should only be used by :class:`Executor` implementations and
        unit tests.
 
+       .. versionchanged:: 3.8
+          This method raises
+          :exc:`concurrent.futures.InvalidStateError` if the :class:`Future` is
+          already done.
 
 Module Functions
 ----------------
@@ -466,6 +475,13 @@ Exception classes
 
    .. versionadded:: 3.7
 
+.. exception:: InvalidStateError
+
+   Raised when an operation is performed on a future that is not allowed
+   in the current state.
+
+   .. versionadded:: 3.8
+
 .. currentmodule:: concurrent.futures.thread
 
 .. exception:: BrokenThreadPool

diff --git a/Lib/asyncio/base_futures.py b/Lib/asyncio/base_futures.py
index 5182884e16d64bf638880463fd2a48b09b4809af..bd65beec553cbcec684cec0e698860d6db91edb2 100644 (file)
--- a/Lib/asyncio/base_futures.py
+++ b/Lib/asyncio/base_futures.py
@@ -1,17 +1,13 @@
 __all__ = ()
 
-import concurrent.futures._base
+import concurrent.futures
 import reprlib
 
 from . import format_helpers
 
-Error = concurrent.futures._base.Error
 CancelledError = concurrent.futures.CancelledError
 TimeoutError = concurrent.futures.TimeoutError
-
-
-class InvalidStateError(Error):
-    """The operation is not allowed in this state."""
+InvalidStateError = concurrent.futures.InvalidStateError
 
 
 # States for Future.

diff --git a/Lib/concurrent/futures/__init__.py b/Lib/concurrent/futures/__init__.py
index 8434fcf4b5ead4b4109a9e75c189dc63e70ad15a..d746aeac50a99763ecce100d17c75fbca123a19d 100644 (file)
--- a/Lib/concurrent/futures/__init__.py
+++ b/Lib/concurrent/futures/__init__.py
@@ -10,6 +10,7 @@ from concurrent.futures._base import (FIRST_COMPLETED,
                                       ALL_COMPLETED,
                                       CancelledError,
                                       TimeoutError,
+                                      InvalidStateError,
                                       BrokenExecutor,
                                       Future,
                                       Executor,

diff --git a/Lib/concurrent/futures/_base.py b/Lib/concurrent/futures/_base.py
index 4f22f7ee0e6d0a33713769e3d3a69fabd08e9b55..d4416c62450e81e14dafa2c183c047b0d019c890 100644 (file)
--- a/Lib/concurrent/futures/_base.py
+++ b/Lib/concurrent/futures/_base.py
@@ -53,6 +53,10 @@ class TimeoutError(Error):
     """The operation exceeded the given deadline."""
     pass
 
+class InvalidStateError(Error):
+    """The operation is not allowed in this state."""
+    pass
+
 class _Waiter(object):
     """Provides the event that wait() and as_completed() block on."""
     def __init__(self):
@@ -513,6 +517,8 @@ class Future(object):
         Should only be used by Executor implementations and unit tests.
         """
         with self._condition:
+            if self._state in {CANCELLED, CANCELLED_AND_NOTIFIED, FINISHED}:
+                raise InvalidStateError('{}: {!r}'.format(self._state, self))
             self._result = result
             self._state = FINISHED
             for waiter in self._waiters:
@@ -526,6 +532,8 @@ class Future(object):
         Should only be used by Executor implementations and unit tests.
         """
         with self._condition:
+            if self._state in {CANCELLED, CANCELLED_AND_NOTIFIED, FINISHED}:
+                raise InvalidStateError('{}: {!r}'.format(self._state, self))
             self._exception = exception
             self._state = FINISHED
             for waiter in self._waiters:

diff --git a/Lib/test/test_concurrent_futures.py b/Lib/test/test_concurrent_futures.py
index b258a0eafde6d4445244083b1af920aca129956b..f2c28ac12b1d09b660862b9829925bf172cb2e9a 100644 (file)
--- a/Lib/test/test_concurrent_futures.py
+++ b/Lib/test/test_concurrent_futures.py
@@ -1206,6 +1206,34 @@ class FutureTests(BaseTestCase):
         self.assertTrue(isinstance(f1.exception(timeout=5), OSError))
         t.join()
 
+    def test_multiple_set_result(self):
+        f = create_future(state=PENDING)
+        f.set_result(1)
+
+        with self.assertRaisesRegex(
+                futures.InvalidStateError,
+                'FINISHED: <Future at 0x[0-9a-f]+ '
+                'state=finished returned int>'
+        ):
+            f.set_result(2)
+
+        self.assertTrue(f.done())
+        self.assertEqual(f.result(), 1)
+
+    def test_multiple_set_exception(self):
+        f = create_future(state=PENDING)
+        e = ValueError()
+        f.set_exception(e)
+
+        with self.assertRaisesRegex(
+                futures.InvalidStateError,
+                'FINISHED: <Future at 0x[0-9a-f]+ '
+                'state=finished raised ValueError>'
+        ):
+            f.set_exception(Exception())
+
+        self.assertEqual(f.exception(), e)
+
 
 @test.support.reap_threads
 def test_main():

diff --git a/Misc/NEWS.d/next/Library/2018-05-24-09-15-52.bpo-33238.ooDfoo.rst b/Misc/NEWS.d/next/Library/2018-05-24-09-15-52.bpo-33238.ooDfoo.rst
new file mode 100644 (file)
index 0000000..b03ab10
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-05-24-09-15-52.bpo-33238.ooDfoo.rst
@@ -0,0 +1,4 @@
+Add ``InvalidStateError`` to :mod:`concurrent.futures`.
+``Future.set_result`` and ``Future.set_exception`` now raise
+``InvalidStateError`` if the futures are not pending or running. Patch by
+Jason Haydaman.