Repository navigation
SystemError in io.BufferedWriter.close when closed errors #140650
Description
Bug report
Bug description:
import io
class MockRawIO:
def __init__(self):
self.closed = NotImplemented
def write(self, b):
return len(data)
def writable(self):
return True
tp = io.BufferedWriter
writer = MockRawIO()
bufio = tp(writer, 8)
bufio.close()SystemError: <method 'close' of '_io.BufferedWriter' objects> returned a result with an exception set
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Activity
inventshah commented on Oct 27, 2025
Bisected to db68bfc. In 3.14, the behavior is emitting a catchable TypeError, but on main it crashes. I believe the problem is the sign of buffered_closed is lost in the IS_CLOSED macro.
cmaloney commented on Oct 27, 2025
What's the use case you have where self.closed does not exist? BufferedReader expects its raw to be a RawIOBase; RawIOBase inherits from IOBase which provides a closed attribute (https://docs.python.org/3/library/io.html#io.IOBase.closed).
I'm 👍on fixing the crash (_io__Buffered_close_impl should return NULL if there is an active exception). Given IS_CLOSED is used in some other macros (ex. CHECK_CLOSED) I'm a little worried about touching it. I think the -1 result from buffered_closed gets coerced to a bool by C rules / 1 so we end up in the if (r > 0) case. Inside that removing the -1 case and only setting res to None if https://docs.python.org/3/c-api/exceptions.html#c.PyErr_Occurred is false I think would be a fairly tactical fix.
inventshah commented on Oct 27, 2025
Actually, in 3.14 only the C implementation of BufferedWriter doesn't crash, the Python implementation does. My PR only restores the 3.14 behavior, so the C implementation does not crash, but the Python implementation still does.
Is it worth trying to patch the Python implementation as well? My feeling is this usage of BufferedWriter is abnormal. I imagine many library facilities may crash if given NotImplemented?
cmaloney commented on Oct 27, 2025
I want to make sure disambiguating crash: "The interpreter exits with an exception / error message" is distinct from "There's a segfault". In this case, the _io C code is misbehaved by what the interpreter loop wants: It returns non-NULL, I believe Py_None, when an exception is set when it should return NULL).
Generally _pyio and _io should match in visible behavior / exceptions (but implementations do differ between the two). In a case like this I'm less worried about them matching because it's not really a supported behavior. Best if they match but not worth a really complex PR or refactor to make that the case.
inventshah commented on Oct 27, 2025
I believe changing IS_CLOSED to preserve the sign for the error check is safe, since its only used in if-condition contexts where C's bool casting rules should apply (the PR), but would you prefer a patch like this?
cmaloney commented on Oct 27, 2025
Yes, that's my preference. Both the CHECK_CLOSED macro and _io_BufferedWriter_write_impl call of IS_CLOSED could set another exception while the "closed exception" is set which I think isn't great (although supported / not a crash; just more exception context)
It might be worthwhile to do a more general fix to CHECK_CLOSED because the "IS_CLOSED" check in it could set an exception (and evaluate to 1) than the && maybe could cause it not error and return NULL itself... I haven't fully investigated that though / need to give it more thought... Also has moderate chance to break other behaviors / tests.
gh-140650: Fix write(), flush() and close() methods of io.BufferedWri…
pythongh-140650: Fix write(), flush() and close() methods of io.Buffe…