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 commentedon 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 ofbuffered_closedis lost in theIS_CLOSEDmacro.cmaloney commentedon Oct 27, 2025
What's the use case you have where
self.closeddoes not exist?BufferedReaderexpects itsrawto be aRawIOBase;RawIOBaseinherits fromIOBasewhich 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_CLOSEDis used in some other macros (ex. CHECK_CLOSED) I'm a little worried about touching it. I think the-1result frombuffered_closedgets coerced to a bool by C rules /1so we end up in theif (r > 0)case. Inside that removing the-1case 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 commentedon Oct 27, 2025
Actually, in 3.14 only the C implementation of
BufferedWriterdoesn'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
BufferedWriteris abnormal. I imagine many library facilities may crash if givenNotImplemented?cmaloney commentedon 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
_ioC 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
_pyioand_ioshould 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 commentedon Oct 27, 2025
I believe changing
IS_CLOSEDto 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 commentedon Oct 27, 2025
Yes, that's my preference. Both the
CHECK_CLOSEDmacro and_io_BufferedWriter_write_implcall ofIS_CLOSEDcould 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_CLOSEDbecause 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…
[-]SystemError in io.BufferedWriter[/-][+]SystemError in io.BufferedWriter.close when closed errors[/+]