classification
Title: multiprocessing.Queue raises AssertionError
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Joseph.Siddall, ZackerySpytz, berker.peksag, davin, jnoller, pitrou, sbt, serhiy.storchaka, xtreak, zvyn
Priority: normal Keywords: patch

Created on 2014-11-14 17:34 by Joseph.Siddall, last changed 2018-10-18 12:55 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
issue22872.patch zvyn, 2015-06-13 05:11 review
Pull Requests
URL Status Linked Edit
PR 9010 merged ZackerySpytz, 2018-08-30 20:50
Messages (7)
msg231164 - (view) Author: Joseph Siddall (Joseph.Siddall) Date: 2014-11-14 17:34
putting something in Queue(multiprocessing.Queue) after closing it raises an AssertionError.
Getting something out of a Queue after closing it raises an OSError.
I expected both scenarios to raise the same exception.


To Reproduce:


>>> from multiprocessing import Queue
>>> q = Queue()
>>> q.close()
>>> q.put("ok")
Traceback (most recent call last):
...
AssertionError


>>> from multiprocessing import Queue
>>> q = Queue()
>>> q.close()
>>> q.get()
Traceback (most recent call last):
...
OSError: handle is closed
msg245291 - (view) Author: Milan Oberkirch (zvyn) * Date: 2015-06-13 05:11
Here is the trivial patch for that :)
msg251178 - (view) Author: Davin Potts (davin) * (Python committer) Date: 2015-09-20 22:21
The proposed patch would potentially break existing code which anticipates the current behavior.  The potential negative impact means this particular proposed patch is not viable.

Choices forward include:  (1) better documenting the existing, established implementation behavior (perhaps having the assert provide a more informative message), or (2) altering the behavior in a new release version (perhaps 3.6 when it comes) and documenting that change appropriately there.

As to what sort of exception we should expect in this situation, unfortunately comparing to the queue module's Queue does not help guide expectations much since it does not have need of a close().

Of the two above options, I'm more inclined towards the first.  Does @Joseph.Siddall have other motivations that helped motivate this suggested improvement?
msg258737 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-01-21 07:20
See issue 5001 for more general cleanup in multiprocessing.

> [...] (2) altering the behavior in a new release version (perhaps 3.6 when it comes) and documenting that change appropriately there.

+1. We can also document AssertionError in older Python versions.
msg313790 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2018-03-13 22:31
The expected behaviour here would be to raise ValueError, as on other closed objects:

>>> s = io.StringIO()
>>> s.close()
>>> s.write("")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: I/O operation on closed file
msg327644 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-10-13 09:26
New changeset 0461704060474cb358d3495322950c4fd00616a0 by Serhiy Storchaka (Zackery Spytz) in branch 'master':
bpo-22872: multiprocessing.Queue's put() and get() now raise ValueError if the queue is closed. (GH-9010)
https://github.com/python/cpython/commit/0461704060474cb358d3495322950c4fd00616a0
msg327979 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-10-18 12:55
I don't think we need to document AssertionError in older Python versions. This is an implementation detail. We don't document all exceptions that can be raised with improper use of the API. Just don't do this.

In addition, using the assert statement for validating user input or the object state which depends on user actions is a bad style. We shouldn't encourage this by documenting it as a behavior of the stdlib. It was a bug, and it is fixed now.
History
Date User Action Args
2018-10-18 12:55:10serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg327979

stage: patch review -> resolved
2018-10-13 09:26:14serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg327644
2018-08-30 20:56:27ZackerySpytzsetnosy: + ZackerySpytz
2018-08-30 20:50:02ZackerySpytzsetstage: needs patch -> patch review
pull_requests: + pull_request8480
2018-08-28 19:47:24xtreaksetnosy: + xtreak
2018-03-13 22:31:22pitrousetcomponents: + Library (Lib), - ctypes
2018-03-13 22:31:13pitrousetnosy: + pitrou

messages: + msg313790
versions: + Python 3.8, - Python 3.6
2018-01-22 21:16:13jqiansetcomponents: + ctypes, - Library (Lib)
versions: - Python 3.4, Python 3.5
2016-01-21 07:20:48berker.peksagsetmessages: + msg258737
stage: needs patch
2015-09-20 22:21:08davinsettype: behavior -> enhancement
messages: + msg251178
stage: patch review -> (no value)
2015-07-21 07:10:09ethan.furmansetnosy: - ethan.furman
2015-06-13 08:53:17berker.peksagsetnosy: + berker.peksag
stage: patch review

versions: + Python 3.5, Python 3.6
2015-06-13 05:11:49zvynsetfiles: + issue22872.patch

nosy: + zvyn
messages: + msg245291

keywords: + patch
2015-02-22 19:51:31davinsetnosy: + davin
2014-11-14 17:35:54ethan.furmansetnosy: + jnoller, ethan.furman, sbt
2014-11-14 17:34:48Joseph.Siddallcreate