homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: PyOS_Readline drops GIL and calls PyOS_StdioReadline, which isn't thread safe
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: 3329 Superseder:
Assigned To: trent Nosy List: gregory.p.smith, kristjan.jonsson, meador.inge, pitrou, python-dev, trent, vstinner
Priority: critical Keywords: patch

Created on 2012年12月21日 10:39 by trent, last changed 2022年04月11日 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
readline_gil.patch vstinner, 2013年06月13日 21:22 review
readline_gil-2.patch vstinner, 2013年06月15日 00:35 review
Messages (10)
msg177874 - (view) Author: Trent Nelson (trent) * (Python committer) Date: 2012年12月21日 10:39
Relevant thread: http://mail.python.org/pipermail/python-dev/2012-December/123225.html
PyOS_StdioReadline features numerous calls that require the GIL to be held. Ideally, the GIL drop-take should be moved closer to the actual underlying read system call.
msg188397 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013年05月04日 18:51
So, could you propose a patch?
msg188499 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2013年05月06日 09:35
My quick and dirty fix is simple:
 _PyOS_ReadlineTState = PyThreadState_GET();
 /* CCP change, cannot release the GIL here because PyOS_StdioReadline uses
 * the regular MALLOC
 */
 /*
 Py_BEGIN_ALLOW_THREADS
 */
#ifdef WITH_THREAD
 PyThread_acquire_lock(_PyOS_ReadlineLock, 1);
#endif
 /* This is needed to handle the unlikely case that the
 * interpreter is in interactive mode *and* stdin/out are not
 * a tty. This can happen, for example if python is run like
 * this: python -i < test1.py
 */
 if (!isatty (fileno (sys_stdin)) || !isatty (fileno (sys_stdout)))
 rv = PyOS_StdioReadline (sys_stdin, sys_stdout, prompt);
 else
 rv = (*PyOS_ReadlineFunctionPointer)(sys_stdin, sys_stdout,
 prompt);
 /*
 Py_END_ALLOW_THREADS
 */
#ifdef WITH_THREAD
 PyThread_release_lock(_PyOS_ReadlineLock);
#endif
Basically, we just comment out the lock release since we don't need it. The reason we found this was that we were using GIL a custom mallocator which should have been run with the GIL but wasn ́t.
msg191092 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013年06月13日 21:22
I just found the readline/GIL issue while working on #18203. I created #18205 but then I found this issue. I just closed #18205 as a duplicate. Here is a patch for Python 3.4.
--
Copy of the initial message (msg191089):
The callback PyOS_ReadlineFunctionPointer (used to read a line from the standard input) must return a buffer allocated by PyMem_Malloc(), but PyOS_Readline() releases the GIL before calling PyOS_ReadlineFunctionPointer.
Simplified extract of PyOS_Readline():
 Py_BEGIN_ALLOW_THREADS
 if (!isatty (fileno (sys_stdin)) || !isatty (fileno (sys_stdout)))
 rv = PyOS_StdioReadline (sys_stdin, sys_stdout, prompt);
 else
 rv = (*PyOS_ReadlineFunctionPointer)(sys_stdin, sys_stdout,
 prompt);
 Py_END_ALLOW_THREADS
tok_nextc() calls PyOS_Readline() and calls PyMem_FREE() to release its result.
PyOS_ReadlineFunctionPointer should allocate memory using malloc(), not using PyMem_Malloc(). But PyOS_Readline() should copy the line into a buffer allocated by PyMem_Malloc() to keep backward compatibility.
See also issue #18203 and #3329.
msg191094 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013年06月13日 22:10
See the following thread on python-dev, the root problem is that PyMem_Malloc() cannot be called with the GIL held. This is a bug in my opinion, and it should be fixed.
http://mail.python.org/pipermail/python-dev/2013-June/126822.html 
msg191178 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013年06月15日 00:35
Updated patch for the final API of #3329. Update also the documentation. PyOS_ReadlineFunctionPointer must now use PyMem_RawMalloc() or PyMem_RawRealloc(), instead of PyMem_Malloc() or PyMem_Realloc().
msg199388 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013年10月10日 14:19
New changeset 98dbe677dfe7 by Victor Stinner in branch 'default':
Close #16742: Fix misuse of memory allocations in PyOS_Readline()
http://hg.python.org/cpython/rev/98dbe677dfe7 
msg200340 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013年10月19日 00:40
New changeset 6c9050ad1afc by Victor Stinner in branch 'default':
Issue #16742: My fix on PyOS_StdioReadline() was incomplete, PyMem_FREE() was
http://hg.python.org/cpython/rev/6c9050ad1afc 
msg200390 - (view) Author: Kristján Valur Jónsson (kristjan.jonsson) * (Python committer) Date: 2013年10月19日 09:48
Perhaps in debug builds the memory apis should verify consistency and matching useage.
msg200404 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013年10月19日 11:34
> Kristján Valur Jónsson added the comment:
>
> Perhaps in debug builds the memory apis should verify consistency and
matching useage.
Python does check usage of apis in debug mode. Memory allocation failure
are almost never checked. See my pyfailmalloc module for that.
History
Date User Action Args
2022年04月11日 14:57:39adminsetgithub: 60946
2013年10月19日 11:34:40vstinnersetmessages: + msg200404
2013年10月19日 09:48:08kristjan.jonssonsetmessages: + msg200390
2013年10月19日 00:40:56python-devsetmessages: + msg200340
2013年10月10日 14:19:38python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg199388

resolution: fixed
stage: needs patch -> resolved
2013年10月01日 11:49:51vstinnerlinkissue18874 dependencies
2013年06月19日 12:03:13vstinnersetdependencies: + API for setting the memory allocator used by Python
2013年06月15日 00:35:50vstinnersetfiles: + readline_gil-2.patch

messages: + msg191178
2013年06月13日 22:10:25vstinnersetmessages: + msg191094
2013年06月13日 22:07:58vstinnerlinkissue18205 superseder
2013年06月13日 21:22:30vstinnersetfiles: + readline_gil.patch
versions: - Python 3.2
nosy: + vstinner

messages: + msg191092

keywords: + patch
2013年05月06日 09:35:21kristjan.jonssonsetmessages: + msg188499
2013年05月04日 18:51:49pitrousetnosy: + pitrou
messages: + msg188397
2013年01月03日 03:16:27meador.ingesetnosy: + meador.inge
2012年12月25日 13:24:07kristjan.jonssonsetnosy: + kristjan.jonsson
2012年12月23日 23:13:06gregory.p.smithsetpriority: normal -> critical
nosy: + gregory.p.smith
2012年12月21日 10:39:50trentcreate

AltStyle によって変換されたページ (->オリジナル) /