Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 35.2k
gh-112292 : Catch import error conditions with readline hooks#112313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
ericsnowcurrently
merged 19 commits into
python:main
from
tonybaloney:robust_readline_hookNov 28, 2023
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
e8d2115
Catch import error conditions with readline callback hook on_startup_…
tonybaloney d56fb47
Add news entry
tonybaloney 6e9e66d
Move the GIL release to after setting an error
tonybaloney 364d8d5
remove else
tonybaloney 4ddf510
Merge branch 'robust_readline_hook' of github.com:tonybaloney/cpython…
tonybaloney 8890f5c
Harden the on_pre_input_hook
tonybaloney f4580c8
Replace uses of a global macro with no error checking with a function…
tonybaloney b408004
Don't set exceptions on non-Python calls.
tonybaloney 30a1a73
Acquire GIL before checking module state since it will require tstate…
tonybaloney 64cfda5
Merge remote-tracking branch 'origin/main' into robust_readline_hook
tonybaloney 451513b
Update Modules/readline.c
tonybaloney 59239f0
Update Modules/readline.c
tonybaloney cf72dc4
Apply suggestions from code review
tonybaloney aa75a62
Update Modules/readline.c
tonybaloney 30c79d7
fixup variable name
tonybaloney ae57bed
Merge branch 'robust_readline_hook' of github.com:tonybaloney/cpython…
tonybaloney b1fa25b
Use simpler assertion
tonybaloney ff038f0
Simpler hook check
tonybaloney 43d609d
Don't assert error check
tonybaloney File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
2 changes: 2 additions & 0 deletions
2 Misc/NEWS.d/next/Library/2023-11-22-19-43-54.gh-issue-112292.5nDU87.rst
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Fix a crash in :mod:`readline` when imported from a sub interpreter. Patch | ||
| by Anthony Shaw |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -147,8 +147,19 @@ readline_free(void *m) | ||
| static PyModuleDef readlinemodule; | ||
| #define readlinestate_global ((readlinestate *)PyModule_GetState(PyState_FindModule(&readlinemodule))) | ||
| static inline readlinestate* | ||
| get_hook_module_state(void) | ||
| { | ||
| PyObject *mod = PyState_FindModule(&readlinemodule); | ||
| if (mod == NULL){ | ||
| PyErr_Clear(); | ||
| return NULL; | ||
| } | ||
| Py_INCREF(mod); | ||
| readlinestate *state = get_readline_state(mod); | ||
| Py_DECREF(mod); | ||
| return state; | ||
| } | ||
| /* Convert to/from multibyte C strings */ | ||
| @@ -438,14 +449,15 @@ readline_set_completion_display_matches_hook_impl(PyObject *module, | ||
| PyObject *function) | ||
| /*[clinic end generated code: output=516e5cb8db75a328 input=4f0bfd5ab0179a26]*/ | ||
| { | ||
| readlinestate *state = get_readline_state(module); | ||
| PyObject *result = set_hook("completion_display_matches_hook", | ||
| &readlinestate_global->completion_display_matches_hook, | ||
| &state->completion_display_matches_hook, | ||
| function); | ||
| #ifdef HAVE_RL_COMPLETION_DISPLAY_MATCHES_HOOK | ||
| /* We cannot set this hook globally, since it replaces the | ||
| default completion display. */ | ||
| rl_completion_display_matches_hook = | ||
| readlinestate_global->completion_display_matches_hook ? | ||
| state->completion_display_matches_hook ? | ||
| #if defined(HAVE_RL_COMPDISP_FUNC_T) | ||
| (rl_compdisp_func_t *)on_completion_display_matches_hook : 0; | ||
| #else | ||
| @@ -472,7 +484,8 @@ static PyObject * | ||
| readline_set_startup_hook_impl(PyObject *module, PyObject *function) | ||
| /*[clinic end generated code: output=02cd0e0c4fa082ad input=7783b4334b26d16d]*/ | ||
| { | ||
| return set_hook("startup_hook", &readlinestate_global->startup_hook, | ||
| readlinestate *state = get_readline_state(module); | ||
| return set_hook("startup_hook", &state->startup_hook, | ||
| function); | ||
| } | ||
| @@ -497,7 +510,8 @@ static PyObject * | ||
| readline_set_pre_input_hook_impl(PyObject *module, PyObject *function) | ||
| /*[clinic end generated code: output=fe1a96505096f464 input=4f3eaeaf7ce1fdbe]*/ | ||
| { | ||
| return set_hook("pre_input_hook", &readlinestate_global->pre_input_hook, | ||
| readlinestate *state = get_readline_state(module); | ||
| return set_hook("pre_input_hook", &state->pre_input_hook, | ||
| function); | ||
| } | ||
| #endif | ||
| @@ -530,7 +544,8 @@ static PyObject * | ||
| readline_get_begidx_impl(PyObject *module) | ||
| /*[clinic end generated code: output=362616ee8ed1b2b1 input=e083b81c8eb4bac3]*/ | ||
| { | ||
| return Py_NewRef(readlinestate_global->begidx); | ||
| readlinestate *state = get_readline_state(module); | ||
| return Py_NewRef(state->begidx); | ||
| } | ||
| /* Get the ending index for the scope of the tab-completion */ | ||
| @@ -545,7 +560,8 @@ static PyObject * | ||
| readline_get_endidx_impl(PyObject *module) | ||
| /*[clinic end generated code: output=7f763350b12d7517 input=d4c7e34a625fd770]*/ | ||
| { | ||
| return Py_NewRef(readlinestate_global->endidx); | ||
| readlinestate *state = get_readline_state(module); | ||
| return Py_NewRef(state->endidx); | ||
| } | ||
| /* Set the tab-completion word-delimiters that readline uses */ | ||
| @@ -772,7 +788,8 @@ static PyObject * | ||
| readline_set_completer_impl(PyObject *module, PyObject *function) | ||
| /*[clinic end generated code: output=171a2a60f81d3204 input=51e81e13118eb877]*/ | ||
| { | ||
| return set_hook("completer", &readlinestate_global->completer, function); | ||
| readlinestate *state = get_readline_state(module); | ||
| return set_hook("completer", &state->completer, function); | ||
| } | ||
| /*[clinic input] | ||
| @@ -785,10 +802,11 @@ static PyObject * | ||
| readline_get_completer_impl(PyObject *module) | ||
| /*[clinic end generated code: output=6e6bbd8226d14475 input=6457522e56d70d13]*/ | ||
| { | ||
| if (readlinestate_global->completer == NULL) { | ||
| readlinestate *state = get_readline_state(module); | ||
| if (state->completer == NULL) { | ||
| Py_RETURN_NONE; | ||
| } | ||
| return Py_NewRef(readlinestate_global->completer); | ||
| return Py_NewRef(state->completer); | ||
| } | ||
| /* Private function to get current length of history. XXX It may be | ||
| @@ -1026,7 +1044,12 @@ on_startup_hook(void) | ||
| { | ||
| int r; | ||
| PyGILState_STATE gilstate = PyGILState_Ensure(); | ||
| r = on_hook(readlinestate_global->startup_hook); | ||
| readlinestate *state = get_hook_module_state(); | ||
| if (state == NULL) { | ||
| PyGILState_Release(gilstate); | ||
| return -1; | ||
| } | ||
| r = on_hook(state->startup_hook); | ||
| PyGILState_Release(gilstate); | ||
| return r; | ||
| } | ||
| @@ -1043,7 +1066,12 @@ on_pre_input_hook(void) | ||
| { | ||
| int r; | ||
| PyGILState_STATE gilstate = PyGILState_Ensure(); | ||
| r = on_hook(readlinestate_global->pre_input_hook); | ||
| readlinestate *state = get_hook_module_state(); | ||
| if (state == NULL) { | ||
| PyGILState_Release(gilstate); | ||
| return -1; | ||
| } | ||
| r = on_hook(state->pre_input_hook); | ||
| PyGILState_Release(gilstate); | ||
| return r; | ||
| } | ||
| @@ -1060,6 +1088,11 @@ on_completion_display_matches_hook(char **matches, | ||
| int i; | ||
| PyObject *sub, *m=NULL, *s=NULL, *r=NULL; | ||
| PyGILState_STATE gilstate = PyGILState_Ensure(); | ||
| readlinestate *state = get_hook_module_state(); | ||
| if (state == NULL) { | ||
| PyGILState_Release(gilstate); | ||
| return; | ||
| } | ||
| m = PyList_New(num_matches); | ||
| if (m == NULL) | ||
| goto error; | ||
| @@ -1070,7 +1103,7 @@ on_completion_display_matches_hook(char **matches, | ||
| PyList_SET_ITEM(m, i, s); | ||
| } | ||
| sub = decode(matches[0]); | ||
| r = PyObject_CallFunction(readlinestate_global->completion_display_matches_hook, | ||
| r = PyObject_CallFunction(state->completion_display_matches_hook, | ||
| "NNi", sub, m, max_length); | ||
| m=NULL; | ||
| @@ -1118,12 +1151,17 @@ static char * | ||
| on_completion(const char *text, int state) | ||
| { | ||
| char *result = NULL; | ||
| if (readlinestate_global->completer != NULL) { | ||
| PyGILState_STATE gilstate = PyGILState_Ensure(); | ||
tonybaloney marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| readlinestate *module_state = get_hook_module_state(); | ||
| if (module_state == NULL) { | ||
| PyGILState_Release(gilstate); | ||
| return NULL; | ||
| } | ||
| if (module_state->completer != NULL) { | ||
| PyObject *r = NULL, *t; | ||
| PyGILState_STATE gilstate = PyGILState_Ensure(); | ||
| rl_attempted_completion_over = 1; | ||
| t = decode(text); | ||
| r = PyObject_CallFunction(readlinestate_global->completer, "Ni", t, state); | ||
| r = PyObject_CallFunction(module_state->completer, "Ni", t, state); | ||
| if (r == NULL) | ||
| goto error; | ||
| if (r == Py_None) { | ||
| @@ -1145,6 +1183,7 @@ on_completion(const char *text, int state) | ||
| PyGILState_Release(gilstate); | ||
| return result; | ||
| } | ||
| PyGILState_Release(gilstate); | ||
| return result; | ||
| } | ||
| @@ -1160,6 +1199,7 @@ flex_complete(const char *text, int start, int end) | ||
| size_t start_size, end_size; | ||
| wchar_t *s; | ||
| PyGILState_STATE gilstate = PyGILState_Ensure(); | ||
| readlinestate *state = get_hook_module_state(); | ||
| #ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER | ||
| rl_completion_append_character ='\0'; | ||
| #endif | ||
| @@ -1187,10 +1227,12 @@ flex_complete(const char *text, int start, int end) | ||
| end = start + (int)end_size; | ||
| done: | ||
| Py_XDECREF(readlinestate_global->begidx); | ||
| Py_XDECREF(readlinestate_global->endidx); | ||
| readlinestate_global->begidx = PyLong_FromLong((long) start); | ||
| readlinestate_global->endidx = PyLong_FromLong((long) end); | ||
| if (state) { | ||
| Py_XDECREF(state->begidx); | ||
| Py_XDECREF(state->endidx); | ||
| state->begidx = PyLong_FromLong((long) start); | ||
| state->endidx = PyLong_FromLong((long) end); | ||
| } | ||
| result = completion_matches((char *)text, *on_completion); | ||
| PyGILState_Release(gilstate); | ||
| return result; | ||
| @@ -1511,12 +1553,17 @@ PyInit_readline(void) | ||
| } | ||
| mod_state = (readlinestate *) PyModule_GetState(m); | ||
| if (mod_state == NULL){ | ||
| goto error; | ||
| } | ||
| PyOS_ReadlineFunctionPointer = call_readline; | ||
| if (setup_readline(mod_state) < 0) { | ||
| PyErr_NoMemory(); | ||
| goto error; | ||
| } | ||
| if (PyErr_Occurred()){ | ||
| goto error; | ||
| } | ||
| return m; | ||
| error: | ||
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.