From b9be010141cd67718d6c1fd48b81f320d8e2490f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 5 Jun 2024 12:30:29 +0200 Subject: [PATCH 1/9] improve doc for `multiprocessing.Queue.empty` --- Doc/library/multiprocessing.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Doc/library/multiprocessing.rst b/Doc/library/multiprocessing.rst index 49762491bae5f4..4949af48fe15da 100644 --- a/Doc/library/multiprocessing.rst +++ b/Doc/library/multiprocessing.rst @@ -837,6 +837,10 @@ For an example of the usage of queues for interprocess communication see Return ``True`` if the queue is empty, ``False`` otherwise. Because of multithreading/multiprocessing semantics, this is not reliable. + If :meth:`put` or :meth:`put_nowait` has been called previously (i.e., + if the feeder thread has been started) and if the queue is closed, this + method raises an :exc:`OSError` instead. + .. method:: full() Return ``True`` if the queue is full, ``False`` otherwise. Because of @@ -940,6 +944,10 @@ For an example of the usage of queues for interprocess communication see Return ``True`` if the queue is empty, ``False`` otherwise. + Unlike :meth:`Queue.empty`, if the queue is closed and this + method is called, this raises an :exc:`OSError` even if the + queue was never used. + .. method:: get() Remove and return an item from the queue. From 7b51da4442297ac7da02485b12271e5992cc79e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 5 Jun 2024 12:33:01 +0200 Subject: [PATCH 2/9] add tests for checking emptiness of queues --- Lib/test/_test_multiprocessing.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 301541a666e140..113dd21ad179e9 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -1202,6 +1202,17 @@ def test_qsize(self): self.assertEqual(q.qsize(), 0) close_queue(q) + def test_empty_exceptions(self): + q = self.Queue() + q.close() # this is a no-op because the feeder thread is not created + self.assertTrue(q.empty()) + close_queue(q) + + q = self.JoinableQueue() + q.close() # this is a no-op because the feeder thread is not created + self.assertTrue(q.empty()) + close_queue(q) + @classmethod def _test_task_done(cls, q): for obj in iter(q.get, None): @@ -5815,6 +5826,12 @@ def _test_empty(cls, queue, child_can_start, parent_can_continue): finally: parent_can_continue.set() + def test_empty_exceptions(self): + q = multiprocessing.SimpleQueue() + q.close() # close the pipe + with self.assertRaisesRegex(OSError, 'is closed'): + q.empty() + def test_empty(self): queue = multiprocessing.SimpleQueue() child_can_start = multiprocessing.Event() From a8d5770fd804c24577941a821baf37b4fce6df6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 5 Jun 2024 12:36:22 +0200 Subject: [PATCH 3/9] blurb --- .../2024-06-05-12-36-18.gh-issue-120012.f14DbQ.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Documentation/2024-06-05-12-36-18.gh-issue-120012.f14DbQ.rst diff --git a/Misc/NEWS.d/next/Documentation/2024-06-05-12-36-18.gh-issue-120012.f14DbQ.rst b/Misc/NEWS.d/next/Documentation/2024-06-05-12-36-18.gh-issue-120012.f14DbQ.rst new file mode 100644 index 00000000000000..6be8dae305482b --- /dev/null +++ b/Misc/NEWS.d/next/Documentation/2024-06-05-12-36-18.gh-issue-120012.f14DbQ.rst @@ -0,0 +1,2 @@ +Clarify the behaviour of :meth:`multiprocessing.Queue.empty` on closed +queues. Patch by Bénédikt Tran. From 07026f3640c6ca33efb0cae9a6d7d4066ac30ea3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 5 Jun 2024 12:59:33 +0200 Subject: [PATCH 4/9] fix tests --- Lib/test/_test_multiprocessing.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 113dd21ad179e9..ad660d0f43be7e 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -1202,17 +1202,6 @@ def test_qsize(self): self.assertEqual(q.qsize(), 0) close_queue(q) - def test_empty_exceptions(self): - q = self.Queue() - q.close() # this is a no-op because the feeder thread is not created - self.assertTrue(q.empty()) - close_queue(q) - - q = self.JoinableQueue() - q.close() # this is a no-op because the feeder thread is not created - self.assertTrue(q.empty()) - close_queue(q) - @classmethod def _test_task_done(cls, q): for obj in iter(q.get, None): @@ -1343,6 +1332,18 @@ def _on_queue_feeder_error(e, obj): self.assertTrue(not_serializable_obj.reduce_was_called) self.assertTrue(not_serializable_obj.on_queue_feeder_error_was_called) + def test_closed_queue_empty_exceptions(self): + for q in multiprocessing.Queue(), multiprocessing.JoinableQueue(): + q.close() # this is a no-op because the feeder thread is not created + self.assertTrue(q.empty()) + + for q in multiprocessing.Queue(), multiprocessing.JoinableQueue(): + q.put('foo') + q.close() # close the internal pipe + with self.assertRaisesRegex(OSError, 'is closed'): + q.empty() + + def test_closed_queue_put_get_exceptions(self): for q in multiprocessing.Queue(), multiprocessing.JoinableQueue(): q.close() From 177e79a352762fdb981c73ba261f72b573ee5a21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 5 Jun 2024 13:25:34 +0200 Subject: [PATCH 5/9] fixup --- Lib/test/_test_multiprocessing.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index ad660d0f43be7e..59cf498359d037 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -1334,16 +1334,17 @@ def _on_queue_feeder_error(e, obj): def test_closed_queue_empty_exceptions(self): for q in multiprocessing.Queue(), multiprocessing.JoinableQueue(): - q.close() # this is a no-op because the feeder thread is not created + q.close() # this is a no-op since the feeder thread is None + q.join_thread() # this is also a no-op self.assertTrue(q.empty()) for q in multiprocessing.Queue(), multiprocessing.JoinableQueue(): q.put('foo') - q.close() # close the internal pipe + q.close() # close the feeder thread + q.join_thread() # make sure to join the feeder thread with self.assertRaisesRegex(OSError, 'is closed'): q.empty() - def test_closed_queue_put_get_exceptions(self): for q in multiprocessing.Queue(), multiprocessing.JoinableQueue(): q.close() From 08985ac6c19f73db9118654499212f158ec81157 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:50:32 +0200 Subject: [PATCH 6/9] add comments --- Lib/test/_test_multiprocessing.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 59cf498359d037..4b3a0645cfc84a 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -1333,13 +1333,17 @@ def _on_queue_feeder_error(e, obj): self.assertTrue(not_serializable_obj.on_queue_feeder_error_was_called) def test_closed_queue_empty_exceptions(self): + # Assert that checking the emptiness of an unused closed queue + # does not raise an OSError. The rationale is that q.close() is + # a no-op upon construction and becomes effective once the queue + # has been used (e.g., by calling q.put()). for q in multiprocessing.Queue(), multiprocessing.JoinableQueue(): q.close() # this is a no-op since the feeder thread is None q.join_thread() # this is also a no-op self.assertTrue(q.empty()) for q in multiprocessing.Queue(), multiprocessing.JoinableQueue(): - q.put('foo') + q.put('foo') # make sure that the queue is 'used' q.close() # close the feeder thread q.join_thread() # make sure to join the feeder thread with self.assertRaisesRegex(OSError, 'is closed'): @@ -5829,6 +5833,9 @@ def _test_empty(cls, queue, child_can_start, parent_can_continue): parent_can_continue.set() def test_empty_exceptions(self): + # Assert that checking emptiness of a closed queue raises + # an OSError, independently of whether the queue was used + # or not. This differs from Queue and JoinableQueue. q = multiprocessing.SimpleQueue() q.close() # close the pipe with self.assertRaisesRegex(OSError, 'is closed'): From 3d150e9cf2ae3a408c3eac57be2e1d57aff442bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Wed, 12 Jun 2024 17:51:28 +0200 Subject: [PATCH 7/9] update NEWS --- .../2024-06-05-12-36-18.gh-issue-120012.f14DbQ.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Documentation/2024-06-05-12-36-18.gh-issue-120012.f14DbQ.rst b/Misc/NEWS.d/next/Documentation/2024-06-05-12-36-18.gh-issue-120012.f14DbQ.rst index 6be8dae305482b..2bf0c977b90387 100644 --- a/Misc/NEWS.d/next/Documentation/2024-06-05-12-36-18.gh-issue-120012.f14DbQ.rst +++ b/Misc/NEWS.d/next/Documentation/2024-06-05-12-36-18.gh-issue-120012.f14DbQ.rst @@ -1,2 +1,3 @@ -Clarify the behaviour of :meth:`multiprocessing.Queue.empty` on closed -queues. Patch by Bénédikt Tran. +Clarify the behaviours of :meth:`multiprocessing.Queue.empty` and +:meth:`multiprocessing.SimpleQueue.empty` on closed queues. +Patch by Bénédikt Tran. From 5bd3f12caa3c5df871ef4534e93c0e80e72f9ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 13 Jun 2024 09:57:32 +0200 Subject: [PATCH 8/9] simplify documentation --- Doc/library/multiprocessing.rst | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Doc/library/multiprocessing.rst b/Doc/library/multiprocessing.rst index 4949af48fe15da..a172ace986b271 100644 --- a/Doc/library/multiprocessing.rst +++ b/Doc/library/multiprocessing.rst @@ -837,9 +837,7 @@ For an example of the usage of queues for interprocess communication see Return ``True`` if the queue is empty, ``False`` otherwise. Because of multithreading/multiprocessing semantics, this is not reliable. - If :meth:`put` or :meth:`put_nowait` has been called previously (i.e., - if the feeder thread has been started) and if the queue is closed, this - method raises an :exc:`OSError` instead. + May raise an :exc:`OSError` on closed queues. .. method:: full() @@ -945,8 +943,7 @@ For an example of the usage of queues for interprocess communication see Return ``True`` if the queue is empty, ``False`` otherwise. Unlike :meth:`Queue.empty`, if the queue is closed and this - method is called, this raises an :exc:`OSError` even if the - queue was never used. + method is called, this raises an :exc:`OSError`. .. method:: get() From 5fefd9c00353f1c318d8f290e7806999518dbaa8 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 13 Jun 2024 11:40:09 -0700 Subject: [PATCH 9/9] Simplify and clarify the doc wording. further doc simplification to alleviate confusion. --- Doc/library/multiprocessing.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Doc/library/multiprocessing.rst b/Doc/library/multiprocessing.rst index a172ace986b271..426291c5f0743d 100644 --- a/Doc/library/multiprocessing.rst +++ b/Doc/library/multiprocessing.rst @@ -837,7 +837,7 @@ For an example of the usage of queues for interprocess communication see Return ``True`` if the queue is empty, ``False`` otherwise. Because of multithreading/multiprocessing semantics, this is not reliable. - May raise an :exc:`OSError` on closed queues. + May raise an :exc:`OSError` on closed queues. (not guaranteed) .. method:: full() @@ -942,8 +942,7 @@ For an example of the usage of queues for interprocess communication see Return ``True`` if the queue is empty, ``False`` otherwise. - Unlike :meth:`Queue.empty`, if the queue is closed and this - method is called, this raises an :exc:`OSError`. + Always raises an :exc:`OSError` if the SimpleQueue is closed. .. method:: get()