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.

Created on 2013-02-24 17:32 by bfroehle, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
readline_completer_state.patch bfroehle, 2013-05-05 22:47 review
Messages (6)
msg182882 - (view) Author: Bradley Froehle (bfroehle) * Date: 2013-02-24 17:32
The `readline.set_completer_delims` doesn't play well with others because it assumes that only it ever allocates or modifies the rl_completer_word_break_characters buffer. If other programs modify this value, for example changing it from heap allocated space to stack allocated space, the results can be catastrophic. To remind you, the function essentially works as: set_completer_delims(PyObject *self, PyObject *args) { // ... free((void*) rl_completer_word_break_characters; rl_completer_word_break_characters = strdup(break_chars); // ... } where `break_chars` is the user provided string. Take, for example, R as another programs which changes the readline completer strings. When an embedded R instance is initialized (say, using `r2py`) something similar to the following takes place:: static void set_rl_completer_word_break_characters(const char *new) { static char[201] buffer; strncpy(buffer, new, 200); rl_completer_word_break_characters = buffer; } static void initialize_embedded_R(...) { // ... set_rl_completer_word_break_characters(...); } As you might expect the next trip through `readline.set_completer_delims` after initializing R will be catastrophic when we attempt to free a stack allocate buffer. I think we should consider modifying the `readline.set_completer_delims` to store the allocated buffers in the module state:: set_completer_delims(PyObject *self, PyObject *args) { // ... free(_readlinestate_global->break_chars); rl_completer_word_break_characters = strdup(break_chars); _readlinestate_global->break_chars = rl_completer_word_break_characters; // ... } This would prevent the segfault and memory leaks, and would render weird hacks (like https://bitbucket.org/lgautier/rpy2/commits/408bae913653 in the r2py code) unnecessary.
msg188412 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-04 21:52
Thanks for reporting this. Do you want to contribute a proper patch? You'll find instructions at http://docs.python.org/devguide/
msg188475 - (view) Author: Bradley Froehle (bfroehle) * Date: 2013-05-05 22:47
Patch attached. I implemented this by adding a 'static char *' which holds the memory we allocate. I did not use the PyState machinery.
msg188577 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-05-06 19:51
New changeset 55c7295aca6c by Antoine Pitrou in branch '2.7': Issue #17289: The readline module now plays nicer with external modules or applications changing the rl_completer_word_break_characters global variable. http://hg.python.org/cpython/rev/55c7295aca6c
msg188578 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-05-06 19:54
New changeset df0afd3ebb70 by Antoine Pitrou in branch '3.3': Issue #17289: The readline module now plays nicer with external modules or applications changing the rl_completer_word_break_characters global variable. http://hg.python.org/cpython/rev/df0afd3ebb70 New changeset 0f65426009e2 by Antoine Pitrou in branch 'default': Issue #17289: The readline module now plays nicer with external modules or applications changing the rl_completer_word_break_characters global variable. http://hg.python.org/cpython/rev/0f65426009e2
msg188579 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-06 19:55
Thanks for the patch! I made a variable name a bit shorter and also added some error checking on the strdup() result.
History
Date User Action Args
2022-04-11 14:57:42adminsetgithub: 61491
2013-05-06 19:55:03pitrousetstatus: open -> closed
resolution: fixed
messages: + msg188579

stage: patch review -> resolved
2013-05-06 19:54:15python-devsetmessages: + msg188578
2013-05-06 19:51:12python-devsetnosy: + python-dev
messages: + msg188577
2013-05-06 19:39:56pitrousetstage: needs patch -> patch review
2013-05-05 22:47:37bfroehlesetfiles: + readline_completer_state.patch
keywords: + patch
messages: + msg188475
2013-05-04 21:52:51pitrousetversions: - Python 3.2
nosy: + pitrou

messages: + msg188412

stage: needs patch
2013-02-26 06:20:46cgohlkesetnosy: + cgohlke
2013-02-26 00:14:37takluyversetnosy: + takluyver
2013-02-24 17:32:42bfroehlecreate