GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-112292 : Catch import error conditions with readline hooks by tonybaloney · Pull Request #112313 · python/cpython

Conversation

tonybaloney commented Nov 22, 2023 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

Prevents a segmentation fault when modules import (or configure) readline from inside a sub interpreter.

The readlinestate_global macro was error-prone to PyImport_FindModule returning NULL and crashing in about 18 places. I could reproduce 1 easily, but this PR replaces the macro with a function and adds error conditions to the other functions.

Issue: Check for sub interpreter support is not catching readline global state and crashing #112292

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with hooray emoji

stonebig reacted with heart emoji

All reactions

🎉 1 reaction
❤️ 1 reaction


          Catch import error conditions with readline callback hook on_startup_…

e8d2115

…hook

bedevere-app Bot mentioned this pull request

Nov 22, 2023

Check for sub interpreter support is not catching readline global state and crashing #112292

Closed

tonybaloney marked this pull request as ready for review

November 22, 2023 08:41

bedevere-app Bot added the awaiting review label

Nov 22, 2023


          Add news entry

d56fb47

corona10 reviewed

Nov 22, 2023

View reviewed changes

Comment thread

Modules/readline.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

corona10 requested a review from ericsnowcurrently

November 22, 2023 10:02

ericsnowcurrently commented Nov 22, 2023

Copy link

Copy Markdown

Member

+1 to fixing the other hooks, whether in this PR or another (though I'd think this PR would be the better place).

stonebig reacted with heart emoji

All reactions

❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ericsnowcurrently requested changes

Nov 22, 2023

View reviewed changes

ericsnowcurrently left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM. I've noted one thing to fix and a couple others to consider.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

stonebig reacted with heart emoji

All reactions

❤️ 1 reaction

Comment thread

Modules/readline.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/readline.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/readline.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/readline.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot added awaiting changes and removed awaiting review labels

Nov 22, 2023

bedevere-app Bot commented Nov 22, 2023

Copy link

Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tonybaloney and others added 7 commits

November 23, 2023 10:29


          Move the GIL release to after setting an error

6e9e66d

Co-authored-by: Eric Snow <[email protected]>


          remove else

364d8d5


          Merge branch 'robust_readline_hook' of github.com:tonybaloney/cpython…

4ddf510

… into robust_readline_hook


          Harden the on_pre_input_hook

8890f5c


          Replace uses of a global macro with no error checking with a function…

f4580c8

… to get global state


          Don't set exceptions on non-Python calls.

b408004


          Acquire GIL before checking module state since it will require tstate…

30a1a73

… checks (won't raise assertion in non-debug code)

tonybaloney changed the title ~~gh-112292 : Catch import error conditions with readline on_startup_hook~~ gh-112292 : Catch import error conditions with readline hooks

Nov 23, 2023

tonybaloney commented Nov 23, 2023

Copy link

Copy Markdown

Contributor Author

I have made the requested changes; please review again

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          Merge remote-tracking branch 'origin/main' into robust_readline_hook

64cfda5

ericsnowcurrently requested changes

Nov 27, 2023

View reviewed changes

ericsnowcurrently left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the extra effort here. There are a few things we may want to do differently here, for which I've left notes.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

There was an error while loading. Please reload this page.

tonybaloney and others added 7 commits

November 28, 2023 10:04


          Update Modules/readline.c

451513b

Co-authored-by: Eric Snow <[email protected]>


          Update Modules/readline.c

59239f0

Co-authored-by: Eric Snow <[email protected]>


          Apply suggestions from code review

cf72dc4

Co-authored-by: Eric Snow <[email protected]>


          Update Modules/readline.c

aa75a62

Co-authored-by: Eric Snow <[email protected]>


          fixup variable name

30c79d7


          Merge branch 'robust_readline_hook' of github.com:tonybaloney/cpython…

ae57bed

… into robust_readline_hook


          Use simpler assertion

b1fa25b

tonybaloney added 2 commits

November 28, 2023 11:21


          Simpler hook check

ff038f0


          Don't assert error check

43d609d

tonybaloney commented Nov 28, 2023

Copy link

Copy Markdown

Contributor Author

@ericsnowcurrently it wasn't possible to check module state inside setup_readline but this code is more robust now so it no longer crashes when imported twice in a sub interpreter. test_builtin and test_readline now correctly fail instead of segfaulting.

$ ./python.exe test_in_interp.py test_builtin
Running test test_builtin
test_abs (test.test_builtin.BuiltinTest.test_abs) ... ok
test_all (test.test_builtin.BuiltinTest.test_all) ... ok
test_any (test.test_builtin.BuiltinTest.test_any) ... ok
test_ascii (test.test_builtin.BuiltinTest.test_ascii) ... ok
test_bin (test.test_builtin.BuiltinTest.test_bin) ... ok
test_bug_27936 (test.test_builtin.BuiltinTest.test_bug_27936) ... ok
test_bytearray_extend_error (test.test_builtin.BuiltinTest.test_bytearray_extend_error) ... ok
test_bytearray_translate (test.test_builtin.BuiltinTest.test_bytearray_translate) ... ok
test_callable (test.test_builtin.BuiltinTest.test_callable) ... ok
test_chr (test.test_builtin.BuiltinTest.test_chr) ... ok
test_cmp (test.test_builtin.BuiltinTest.test_cmp) ... ok
test_compile (test.test_builtin.BuiltinTest.test_compile) ... ok
test_compile_ast (test.test_builtin.BuiltinTest.test_compile_ast) ... ok
test_compile_async_generator (test.test_builtin.BuiltinTest.test_compile_async_generator)
With the PyCF_ALLOW_TOP_LEVEL_AWAIT flag added in 3.8, we want to ... ok
test_compile_top_level_await (test.test_builtin.BuiltinTest.test_compile_top_level_await)
Test whether code some top level await can be compiled. ... ok
test_compile_top_level_await_invalid_cases (test.test_builtin.BuiltinTest.test_compile_top_level_await_invalid_cases) ... ok
test_compile_top_level_await_no_coro (test.test_builtin.BuiltinTest.test_compile_top_level_await_no_coro)
Make sure top level non-await codes get the correct coroutine flags ... ok
test_construct_singletons (test.test_builtin.BuiltinTest.test_construct_singletons) ... ok
test_delattr (test.test_builtin.BuiltinTest.test_delattr) ... ok
test_dir (test.test_builtin.BuiltinTest.test_dir) ... ok
test_divmod (test.test_builtin.BuiltinTest.test_divmod) ... ok
test_eval (test.test_builtin.BuiltinTest.test_eval) ... ok
test_exec (test.test_builtin.BuiltinTest.test_exec) ... ok
test_exec_closure (test.test_builtin.BuiltinTest.test_exec_closure) ... ok
test_exec_globals (test.test_builtin.BuiltinTest.test_exec_globals) ... ok
test_exec_globals_dict_subclass (test.test_builtin.BuiltinTest.test_exec_globals_dict_subclass) ... ok
test_exec_globals_error_on_get (test.test_builtin.BuiltinTest.test_exec_globals_error_on_get) ... ok
test_exec_globals_frozen (test.test_builtin.BuiltinTest.test_exec_globals_frozen) ... ok
test_exec_redirected (test.test_builtin.BuiltinTest.test_exec_redirected) ... ok
test_filter (test.test_builtin.BuiltinTest.test_filter) ... ok
test_filter_dealloc (test.test_builtin.BuiltinTest.test_filter_dealloc) ... skipped "resource 'cpu' is not enabled"
test_filter_pickle (test.test_builtin.BuiltinTest.test_filter_pickle) ... ok
test_format (test.test_builtin.BuiltinTest.test_format) ... ok
test_general_eval (test.test_builtin.BuiltinTest.test_general_eval) ... ok
test_getattr (test.test_builtin.BuiltinTest.test_getattr) ... ok
test_hasattr (test.test_builtin.BuiltinTest.test_hasattr) ... ok
test_hash (test.test_builtin.BuiltinTest.test_hash) ... ok
test_hex (test.test_builtin.BuiltinTest.test_hex) ... ok
test_id (test.test_builtin.BuiltinTest.test_id) ... ok
test_import (test.test_builtin.BuiltinTest.test_import) ... ok
test_input (test.test_builtin.BuiltinTest.test_input) ... ok
test_isinstance (test.test_builtin.BuiltinTest.test_isinstance) ... ok
test_issubclass (test.test_builtin.BuiltinTest.test_issubclass) ... ok
test_iter (test.test_builtin.BuiltinTest.test_iter) ... ok
test_len (test.test_builtin.BuiltinTest.test_len) ... ok
test_map (test.test_builtin.BuiltinTest.test_map) ... ok
test_map_pickle (test.test_builtin.BuiltinTest.test_map_pickle) ... ok
test_max (test.test_builtin.BuiltinTest.test_max) ... ok
test_min (test.test_builtin.BuiltinTest.test_min) ... ok
test_neg (test.test_builtin.BuiltinTest.test_neg) ... ok
test_next (test.test_builtin.BuiltinTest.test_next) ... ok
test_oct (test.test_builtin.BuiltinTest.test_oct) ... ok
test_open (test.test_builtin.BuiltinTest.test_open) ... ok
test_open_default_encoding (test.test_builtin.BuiltinTest.test_open_default_encoding) ... ok
test_open_non_inheritable (test.test_builtin.BuiltinTest.test_open_non_inheritable) ... ok
test_ord (test.test_builtin.BuiltinTest.test_ord) ... ok
test_pow (test.test_builtin.BuiltinTest.test_pow) ... ok
test_repr (test.test_builtin.BuiltinTest.test_repr) ... ok
test_round (test.test_builtin.BuiltinTest.test_round) ... ok
test_round_large (test.test_builtin.BuiltinTest.test_round_large) ... ok
test_setattr (test.test_builtin.BuiltinTest.test_setattr) ... ok
test_sum (test.test_builtin.BuiltinTest.test_sum) ... ok
test_sum_accuracy (test.test_builtin.BuiltinTest.test_sum_accuracy) ... ok
test_type (test.test_builtin.BuiltinTest.test_type) ... ok
test_vars (test.test_builtin.BuiltinTest.test_vars) ... ok
test_warning_notimplemented (test.test_builtin.BuiltinTest.test_warning_notimplemented) ... ok
test_zip (test.test_builtin.BuiltinTest.test_zip) ... ok
test_zip_bad_iterable (test.test_builtin.BuiltinTest.test_zip_bad_iterable) ... ok
test_zip_pickle (test.test_builtin.BuiltinTest.test_zip_pickle) ... ok
test_zip_pickle_strict (test.test_builtin.BuiltinTest.test_zip_pickle_strict) ... ok
test_zip_pickle_strict_fail (test.test_builtin.BuiltinTest.test_zip_pickle_strict_fail) ... ok
test_zip_result_gc (test.test_builtin.BuiltinTest.test_zip_result_gc) ... ok
test_zip_strict (test.test_builtin.BuiltinTest.test_zip_strict) ... ok
test_zip_strict_error_handling (test.test_builtin.BuiltinTest.test_zip_strict_error_handling) ... ok
test_zip_strict_error_handling_stopiteration (test.test_builtin.BuiltinTest.test_zip_strict_error_handling_stopiteration) ... ok
test_zip_strict_iterators (test.test_builtin.BuiltinTest.test_zip_strict_iterators) ... ok
test_immortals (test.test_builtin.ImmortalTests.test_immortals) ... ok
test_list_repeat_respect_immortality (test.test_builtin.ImmortalTests.test_list_repeat_respect_immortality) ... ok
test_tuple_repeat_respect_immortality (test.test_builtin.ImmortalTests.test_tuple_repeat_respect_immortality) ... ok
test_input_no_stdout_fileno (test.test_builtin.PtyTests.test_input_no_stdout_fileno) ... ERROR
test_input_tty (test.test_builtin.PtyTests.test_input_tty) ... ERROR
test_input_tty_non_ascii (test.test_builtin.PtyTests.test_input_tty_non_ascii) ... ERROR
test_input_tty_non_ascii_unicode_errors (test.test_builtin.PtyTests.test_input_tty_non_ascii_unicode_errors) ... ERROR
test_cleanup (test.test_builtin.ShutdownTest.test_cleanup) ... ok
test_breakpoint (test.test_builtin.TestBreakpoint.test_breakpoint) ... ok
test_breakpoint_with_args_and_keywords (test.test_builtin.TestBreakpoint.test_breakpoint_with_args_and_keywords) ... ok
test_breakpoint_with_breakpointhook_reset (test.test_builtin.TestBreakpoint.test_breakpoint_with_breakpointhook_reset) ... ok
test_breakpoint_with_breakpointhook_set (test.test_builtin.TestBreakpoint.test_breakpoint_with_breakpointhook_set) ... ok
test_breakpoint_with_passthru_error (test.test_builtin.TestBreakpoint.test_breakpoint_with_passthru_error) ... ok
test_envar_good_path_builtin (test.test_builtin.TestBreakpoint.test_envar_good_path_builtin) ... ok
test_envar_good_path_empty_string (test.test_builtin.TestBreakpoint.test_envar_good_path_empty_string) ... ok
test_envar_good_path_noop_0 (test.test_builtin.TestBreakpoint.test_envar_good_path_noop_0) ... ok
test_envar_good_path_other (test.test_builtin.TestBreakpoint.test_envar_good_path_other) ... ok
test_envar_ignored_when_hook_is_set (test.test_builtin.TestBreakpoint.test_envar_ignored_when_hook_is_set) ... ok
test_envar_unimportable (test.test_builtin.TestBreakpoint.test_envar_unimportable) ... ok
test_runtime_error_when_hook_is_lost (test.test_builtin.TestBreakpoint.test_runtime_error_when_hook_is_lost) ... ok
test_bad_arguments (test.test_builtin.TestSorted.test_bad_arguments) ... ok
test_baddecorator (test.test_builtin.TestSorted.test_baddecorator) ... ok
test_basic (test.test_builtin.TestSorted.test_basic) ... ok
test_inputtypes (test.test_builtin.TestSorted.test_inputtypes) ... ok
test_bad_args (test.test_builtin.TestType.test_bad_args) ... ok
test_bad_slots (test.test_builtin.TestType.test_bad_slots) ... ok
test_namespace_order (test.test_builtin.TestType.test_namespace_order) ... ok
test_new_type (test.test_builtin.TestType.test_new_type) ... ok
test_type_doc (test.test_builtin.TestType.test_type_doc) ... ok
test_type_name (test.test_builtin.TestType.test_type_name) ... ok
test_type_nokwargs (test.test_builtin.TestType.test_type_nokwargs) ... ok
test_type_qualname (test.test_builtin.TestType.test_type_qualname) ... ok
test_type_typeparams (test.test_builtin.TestType.test_type_typeparams) ... ok
bin (builtins)
Doctest: builtins.bin ... ok
hex (builtins.bytearray)
Doctest: builtins.bytearray.hex ... ok
hex (builtins.bytes)
Doctest: builtins.bytes.hex ... ok
as_integer_ratio (builtins.float)
Doctest: builtins.float.as_integer_ratio ... ok
fromhex (builtins.float)
Doctest: builtins.float.fromhex ... ok
hex (builtins.float)
Doctest: builtins.float.hex ... ok
hex (builtins)
Doctest: builtins.hex ... ok
int (builtins)
Doctest: builtins.int ... ok
as_integer_ratio (builtins.int)
Doctest: builtins.int.as_integer_ratio ... ok
bit_count (builtins.int)
Doctest: builtins.int.bit_count ... ok
bit_length (builtins.int)
Doctest: builtins.int.bit_length ... ok
hex (builtins.memoryview)
Doctest: builtins.memoryview.hex ... ok
oct (builtins)
Doctest: builtins.oct ... ok
zip (builtins)
Doctest: builtins.zip ... ok

======================================================================
ERROR: test_input_no_stdout_fileno (test.test_builtin.PtyTests.test_input_no_stdout_fileno)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2314, in test_input_no_stdout_fileno
    lines = self.run_child(child, b"quux\r")
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2187, in run_child
    old_sighup = signal.signal(signal.SIGHUP, self.handle_sighup)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/anthonyshaw/projects/cpython/Lib/signal.py", line 56, in signal
    handler = _signal.signal(_enum_to_int(signalnum), _enum_to_int(handler))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: signal only works in main thread of the main interpreter

======================================================================
ERROR: test_input_tty (test.test_builtin.PtyTests.test_input_tty)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2284, in test_input_tty
    self.check_input_tty("prompt", b"quux")
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2269, in check_input_tty
    lines = self.run_child(child, terminal_input + b"\r\n")
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2187, in run_child
    old_sighup = signal.signal(signal.SIGHUP, self.handle_sighup)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/anthonyshaw/projects/cpython/Lib/signal.py", line 56, in signal
    handler = _signal.signal(_enum_to_int(signalnum), _enum_to_int(handler))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: signal only works in main thread of the main interpreter

======================================================================
ERROR: test_input_tty_non_ascii (test.test_builtin.PtyTests.test_input_tty_non_ascii)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2299, in test_input_tty_non_ascii
    self.check_input_tty("prompté", b"quux\xe9", "utf-8")
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2269, in check_input_tty
    lines = self.run_child(child, terminal_input + b"\r\n")
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2187, in run_child
    old_sighup = signal.signal(signal.SIGHUP, self.handle_sighup)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/anthonyshaw/projects/cpython/Lib/signal.py", line 56, in signal
    handler = _signal.signal(_enum_to_int(signalnum), _enum_to_int(handler))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: signal only works in main thread of the main interpreter

======================================================================
ERROR: test_input_tty_non_ascii_unicode_errors (test.test_builtin.PtyTests.test_input_tty_non_ascii_unicode_errors)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2304, in test_input_tty_non_ascii_unicode_errors
    self.check_input_tty("prompté", b"quux\xe9", "ascii")
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2269, in check_input_tty
    lines = self.run_child(child, terminal_input + b"\r\n")
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/anthonyshaw/projects/cpython/Lib/test/test_builtin.py", line 2187, in run_child
    old_sighup = signal.signal(signal.SIGHUP, self.handle_sighup)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/anthonyshaw/projects/cpython/Lib/signal.py", line 56, in signal
    handler = _signal.signal(_enum_to_int(signalnum), _enum_to_int(handler))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: signal only works in main thread of the main interpreter

----------------------------------------------------------------------
Ran 122 tests in 0.298s

FAILED (errors=4, skipped=1)
test test_builtin failed
$ ./python.exe test_in_interp.py test_readline
Running test test_readline
test_readline skipped -- module readline does not support loading in subinterpreters

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ericsnowcurrently commented Nov 28, 2023

Copy link

Copy Markdown

Member

it wasn't possible to check module state inside setup_readline

I'm guessing it's because PyState_FindModule() will always return NULL until after the module init func has already finished. That's fine.

but this code is more robust now so it no longer crashes when imported twice in a sub interpreter.

Yeah, that's the higher priority thing. The module is definitely in a better place now.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ericsnowcurrently commented Nov 28, 2023

Copy link

Copy Markdown

Member

How much of this should be backported to 3.12?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tonybaloney commented Nov 28, 2023

Copy link

Copy Markdown

Contributor Author

How much of this should be backported to 3.12?

All of it could be

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tonybaloney deleted the robust_readline_hook branch

November 28, 2023 02:01

ericsnowcurrently commented Nov 28, 2023

Copy link

Copy Markdown

Member

@Yhg1s, what do you think about backporting this as-is?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aisk pushed a commit to aisk/cpython that referenced this pull request

Feb 11, 2024


          pythongh-112292 : Catch import error conditions with readline hooks (p…

153b1c2

…ythongh-112313)

Prevents a segmentation fault in registered hooks for the readline library, but only when the readline module is loaded inside an isolated sub interpreter.  The module is single-phase init so loading it fails, but not until the module init function has already run, where the readline hooks get registered.

The readlinestate_global macro was error-prone to PyImport_FindModule returning NULL and crashing in about 18 places.  I could reproduce 1 easily, but this PR replaces the macro with a function and adds error conditions to the other functions.

ericsnowcurrently mentioned this pull request

Jul 8, 2024

[subinterpreters] Multiple interpreters and readline module hook functions. #48452

Closed

Glyphack pushed a commit to Glyphack/cpython that referenced this pull request

Sep 2, 2024


          pythongh-112292 : Catch import error conditions with readline hooks (p…

4536a05

…ythongh-112313)

Prevents a segmentation fault in registered hooks for the readline library, but only when the readline module is loaded inside an isolated sub interpreter.  The module is single-phase init so loading it fails, but not until the module init function has already run, where the readline hooks get registered.

The readlinestate_global macro was error-prone to PyImport_FindModule returning NULL and crashing in about 18 places.  I could reproduce 1 easily, but this PR replaces the macro with a function and adds error conditions to the other functions.

Reviewers

corona10

corona10 left review comments

ericsnowcurrently

ericsnowcurrently approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

tonybaloney commented Nov 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ericsnowcurrently commented Nov 22, 2023

Uh oh!

ericsnowcurrently left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-app Bot commented Nov 22, 2023

Uh oh!

tonybaloney commented Nov 23, 2023

Uh oh!

ericsnowcurrently left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tonybaloney commented Nov 28, 2023

Uh oh!

ericsnowcurrently commented Nov 28, 2023

Uh oh!

ericsnowcurrently left a comment

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently commented Nov 28, 2023

Uh oh!

ericsnowcurrently commented Nov 28, 2023

Uh oh!

tonybaloney commented Nov 28, 2023

Uh oh!

ericsnowcurrently commented Nov 28, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tonybaloney commented Nov 22, 2023 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.