potential fix for windows interrupts #1753
Conversation
|
Many thanks @kevinushey! I was really stumped on the issue with interrupts on Windows in Rstudio IDE, and this helps tremendously. Do you think it's ready to merge? |
src/libpython.cpp
Outdated
| else | ||
| tmp = PyObject_CallMethod(sys_stdout, "flush", NULL); | ||
| // try to invoke flush method | ||
| if (!PyObject_HasAttrString(buffer, "flush")) |
There was a problem hiding this comment.
This probably doesn't matter, but I noticed in the python sources that PyObject_HasAttr() just calls PyObject_GetAttr() and then checks if an exception was raised. It's trivial, but we're on a hot code path here and the approach seems unnecessarily expensive if we're going to immediately fetch the attr again right after this in the common case, and we've already established an error-catching scope.
More relevant however, is that flush should always be present.
sys.stdout and sys.stderr are supposed to be a file object, and the abstract io base class defines this mixin method always: https://docs.python.org/3/library/io.html#io.IOBase (the method does nothing for output streams where flushing doesn't apply)
If flush missing, I think we should emit some error output, because it indicates that something is "wrong".
There was a problem hiding this comment.
FWIW we have this code that apparently expects stdout to be null in some scenarios.
https://github.com/rstudio/reticulate/blob/bugfix/windows-interrupts/R/output.R#L9-L13
|
Thank you very much! |
No description provided.