gh-155907: Raise OSError and KeyboardInterrupt when reading marshal data

[serhiy-storchaka]

r_string(), r_byte() and PyMarshal_ReadLastObjectFromFile() did not check ferror() nor call PyErr_CheckSignals(), so a genuine I/O error or a read interrupted by Ctrl-C was reported as EOFError, as if the file had simply ended.

>>> import _testcapi, os
>>> os.mkdir('d')
>>> _testcapi.pymarshal_read_object_from_file('d')
EOFError: EOF read where object expected      # before
IsADirectoryError: [Errno 21] Is a directory  # after

The writing side had the same gap: w_flush() and w_string() discarded the result of fwrite(), so a failed or interrupted write to a FILE* was ignored. PyMarshal_WriteObjectToFile() also never turned the recorded WFERR_* code into an exception, so even an unmarshallable value was ignored.

>>> import _testcapi, marshal
>>> _testcapi.pymarshal_write_object_to_file(b'x' * 100000, '/dev/full', marshal.version)
                                             # before: no error
OSError: [Errno 28] No space left on device  # after
>>> _testcapi.pymarshal_write_object_to_file(object(), 'f', marshal.version)
                                             # before: no error
ValueError: unmarshallable object            # after

The WFERR_* to exception mapping is factored out of PyMarshal_WriteObjectToString() into w_set_error(), and all three writers now use it. This removes the note above PyMarshal_WriteLongToFile() saying that it never sets an exception and that doing so "should be regarded as an API-breaking change": detecting write errors makes it set one, which is what its documentation has promised since gh-105184.

The documentation of these functions listed only EOFError, ValueError and TypeError, which was inaccurate even before this change (MemoryError is raised when the read buffer cannot be allocated), so the per-function lists are replaced by one list for the whole section.

• Issue: marshal reports EOFError for I/O errors when reading from a FILE* #155907

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #34392700 | 📁 Comparing 01413ca against main (7a845ce)

  🔍 Preview build  

77 files changed · + 1 added · ± 76 modified

+ Added

• library/time-complexity.html

± Modified

• glossary.html
• c-api/import.html
• c-api/marshal.html
• c-api/veryhigh.html
• faq/design.html
• faq/programming.html
• howto/a-conceptual-overview-of-asyncio.html
• howto/curses.html
• howto/functional.html
• library/ast.html
• and 66 more...

[serhiy-storchaka]

Failure on Windows is expected -- this is a bug #155905.

[vstinner]

marshal.load() and marshal.dump() error handling also changes and should be documented in Doc/library/marshal.rst.

[serhiy-storchaka]

No, they are not affected. They do not use FILE* based C API.

[vstinner]

I checked again your change, you're right and I'm wrong. marshal.load() and marshal.dump() are not affected.

[vstinner]

LGTM. I just suggest renaming WFERR_ERROR_SET to WFERR_EXCEPTION_SET.

[vstinner]

You should rename the constant WFERR_EXCEPTION_SET since it's an "exception set", not an "error set".

[vstinner]

I checked again your change, you're right and I'm wrong. marshal.load() and marshal.dump() are not affected.

[vstinner]

vstinner removed needs backport to 3.14 needs backport to 3.15 labels

I don't think that it's a good idea to backport this change. Some projects can rely on the current exact exception raised by these C functions. It's ok to change them in Python 3.16, but IMO it's too risky to backport the change to stable versions.

[serhiy-storchaka]

Thank you for your review. Initially it was smaller change, but it grew more after discovering more silenced/overridden errors. I plannet to fix the writing part in a separate issue, but is better for documentation if fix them together.