[3.9] bpo-40826: Fix GIL usage in PyOS_Readline() #20613
Conversation
my_fgets() now calls _PyOS_InterruptOccurred(tstate) to check for pending signals, rather calling PyOS_InterruptOccurred(). my_fgets() is called with the GIL released, whereas PyOS_InterruptOccurred() must be called with the GIL held. test_repl: use text=True and avoid SuppressCrashReport in test_multiline_string_parsing(). Fix my_fgets() on Windows: fgets(fp) does crash if fileno(fp) is closed. (cherry picked from commit fa7ab6a)
|
I validated manually on Linux and Windows that this change fix https://bugs.python.org/issue40826 using the interactive REPL: type |
* bpo-40826: Fix GIL usage in PyOS_Readline() (GH-20579) Fix GIL usage in PyOS_Readline(): lock the GIL to set an exception. Pass tstate to my_fgets() and _PyOS_WindowsConsoleReadline(). Cleanup these functions. (cherry picked from commit c353764) * bpo-40826: Add _PyOS_InterruptOccurred(tstate) function (GH-20599) my_fgets() now calls _PyOS_InterruptOccurred(tstate) to check for pending signals, rather calling PyOS_InterruptOccurred(). my_fgets() is called with the GIL released, whereas PyOS_InterruptOccurred() must be called with the GIL held. test_repl: use text=True and avoid SuppressCrashReport in test_multiline_string_parsing(). Fix my_fgets() on Windows: fgets(fp) does crash if fileno(fp) is closed. (cherry picked from commit fa7ab6a)
| /* bpo-40826: fgets(fp) does crash if fileno(fp) is closed */ | ||
| if (handle == INVALID_HANDLE_VALUE) { | ||
| return -1; /* EOF */ | ||
| } |
There was a problem hiding this comment.
_get_osfhandle sets errno to EBADF if the fd is closed, so this is an error case that should return -2. That's consistent with Linux fgets for this case.
There was a problem hiding this comment.
my_fgets() is not a generic wrapper for fgets(), but a helper function for PyOS_StdioReadline() which doesn't use errno.
There was a problem hiding this comment.
If you see a bug in my change, please comment https://bugs.python.org/issue40826.
There was a problem hiding this comment.
I just thought it would be better to be consistent with the function's API -- returning -2 for an error -- and to be consistent how the same case would behave in Linux. Also, to not include a misleading /* EOF */ comment for a case that has nothing to do with EOF.
There was a problem hiding this comment.
Well... the caller handles -1 and -2 the same way :-) This PR is a backport to 3.9. If you want to adjust the behavior, you can propose a PR on the master branch ;-)
https://bugs.python.org/issue40826