gh-105184: document that marshal functions can fail and need to be checked with PyErr_Occurred

[iritkatriel]

@encukou

As you suggested, I made this PR in order to backport it as far back as we can. Both of these functions should be changed to have int return values.

I have one doubt though about what I did here - PyMarshal_WriteLongToFile, as far as I can tell, doesn't actually ever fail. But we don't want to document that, because we want to change it to have int return value and be fallible (in case of future changes). So I think we do want to say in the doc that it can fail, even though now it can't. What do you think?

• Issue: C API functions with no return value cannot report errors #105184

---

📚 Documentation preview 📚: https://cpython-previews--105185.org.readthedocs.build/

[encukou]

I have one doubt though about what I did here - PyMarshal_WriteLongToFile, as far as I can tell, doesn't actually ever fail. But we don't want to document that, because we want to change it to have int return value and be fallible (in case of future changes). So I think we do want to say in the doc that it can fail, even though now it can't. What do you think?

My thinking is a bit nuanced :)

For the user-facing docs, definitely say that the error should be checked. Your wording is perfect.

But, we should assume that authors of existing code use the existing documentation. So, the return value change should be treated as an API break despite what current documentation will say.
There should be a comment in the source to warn future devs about that.

[iritkatriel]

There should be a comment in the source to warn future devs about that.

Agreed.

It doesn't make sense to add it to this PR, because for now the return value is void. But in the PR that makes it int I will add this comment.

[iritkatriel]

There should be a comment in the source to warn future devs about that.

Agreed.

It doesn't make sense to add it to this PR, because for now the return value is void. But in the PR that makes it int I will add this comment.

Actually I can write a comment here that setting an error is a breaking change.

[miss-islington]

Thanks @iritkatriel for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8, 3.9, 3.10, 3.11, 3.12.
🐍🍒⛏🤖

[bedevere-bot]

GH-105218 is a backport of this pull request to the 3.12 branch.

[bedevere-bot]

GH-105219 is a backport of this pull request to the 3.11 branch.

[bedevere-bot]

GH-105220 is a backport of this pull request to the 3.10 branch.

[bedevere-bot]

GH-105221 is a backport of this pull request to the 3.9 branch.

[bedevere-bot]

GH-105222 is a backport of this pull request to the 3.8 branch.

[bedevere-bot]

GH-105223 is a backport of this pull request to the 3.7 branch.

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM64 Windows Non-Debug 3.x has failed when building commit ee26ca1.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/730/builds/8103) and take a look at the build logs.
4. Check if the failure is related to this commit (ee26ca1) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/730/builds/8103

Failed tests:

• test_subprocess

Summary of the results of the build (if available):

== Tests result: FAILURE then SUCCESS ==

415 tests OK.

10 slowest tests:

• test_peg_generator: 3 min 59 sec
• test_multiprocessing_spawn: 2 min 13 sec
• test_concurrent_futures: 1 min 16 sec
• test_socket: 1 min 7 sec
• test_mmap: 1 min 1 sec
• test_io: 56.4 sec
• test_math: 54.7 sec
• test_hashlib: 46.9 sec
• test_urllib2_localnet: 45.4 sec
• test_tarfile: 36.5 sec

32 tests skipped:
test.test_asyncio.test_unix_events test_curses test_dbm_gnu
test_dbm_ndbm test_devpoll test_epoll test_fcntl test_fork1
test_gdb test_grp test_ioctl test_kqueue test_multiprocessing_fork
test_multiprocessing_forkserver test_openpty test_perf_profiler
test_perfmaps test_poll test_posix test_pty test_pwd test_readline
test_resource test_syslog test_threadsignals test_tkinter test_ttk
test_wait3 test_wait4 test_xxtestfuzz test_zipfile64 test_zoneinfo

1 re-run test:
test_subprocess

Total duration: 20 min 39 sec

Click to see traceback logs

Traceback (most recent call last): File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\support\__init__.py", line 207, in _force_runreturn func(*args) ^^^^^^^^^^^PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Workspace\\buildarea\\3.x.linaro-win-arm64.nondebug\\build\\build\\test_python_12184�\\test_python_worker_8132�' Traceback (most recent call last): File "<frozen runpy>", line 198, in _run_module_as_main File "<frozen runpy>", line 88, in _run_code File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\__main__.py", line 2, in <module> main() File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\libregrtest\main.py", line 802, in main Regrtest().main(tests=tests, **kwargs) File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\libregrtest\main.py", line 732, in mainwith os_helper.temp_cwd(test_cwd, quiet=True): File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\contextlib.py", line 155, in __exit__self.gen.throw(value) File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\support\os_helper.py", line 531, in temp_cwdwith temp_dir(path=name, quiet=quiet) as temp_path: File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\contextlib.py", line 155, in __exit__self.gen.throw(value) File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\support\os_helper.py", line 485, in temp_dir rmtree(path) File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\support\os_helper.py", line 442, in rmtree _rmtree(path) File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\support\os_helper.py", line 385, in _rmtree _waitfor(_rmtree_inner, path, waitall=True) File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\support\os_helper.py", line 330, in _waitfor func(pathname) File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\support\os_helper.py", line 382, in _rmtree_inner _force_run(fullname, os.rmdir, fullname) File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\support\__init__.py", line 218, in _force_runreturn func(*args) ^^^^^^^^^^^PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Workspace\\buildarea\\3.x.linaro-win-arm64.nondebug\\build\\build\\test_python_12184�\\test_python_worker_8132�' Traceback (most recent call last): File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\support\os_helper.py", line 480, in temp_diryield path File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\support\os_helper.py", line 533, in temp_cwdyield cwd_dir File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\libregrtest\main.py", line 738, in mainself._main(tests, kwargs) File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\test\libregrtest\main.py", line 797, in _main sys.exit(0) SystemExit: 0

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Arch Linux TraceRefs 3.x has failed when building commit ee26ca1.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/484/builds/3433) and take a look at the build logs.
4. Check if the failure is related to this commit (ee26ca1) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/484/builds/3433

Failed tests:

• test__xxsubinterpreters

Summary of the results of the build (if available):

==

Click to see traceback logs

remote: Enumerating objects: 7, done. remote: Counting objects: 14% (1/7) remote: Counting objects: 28% (2/7) remote: Counting objects: 42% (3/7) remote: Counting objects: 57% (4/7) remote: Counting objects: 71% (5/7) remote: Counting objects: 85% (6/7) remote: Counting objects: 100% (7/7) remote: Counting objects: 100% (7/7), done. remote: Compressing objects: 14% (1/7) remote: Compressing objects: 28% (2/7) remote: Compressing objects: 42% (3/7) remote: Compressing objects: 57% (4/7) remote: Compressing objects: 71% (5/7) remote: Compressing objects: 85% (6/7) remote: Compressing objects: 100% (7/7) remote: Compressing objects: 100% (7/7), done. remote: Total 7 (delta 0), reused 2 (delta 0), pack-reused 0  From https://github.com/python/cpython * branch main -> FETCH_HEAD Note: switching to 'ee26ca13a129da8cf549409d0a1b2e892ff2b4ec'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by switching back to a branch. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -c with the switch command. Example: git switch -c <new-branch-name> Or undo this operation with: git switch - Turn off this advice by setting config variable advice.detachedHead to false HEAD is now at ee26ca13a1 gh-105184: document that marshal functions can fail and need to be checked with PyErr_Occurred (#105185) Switched to and reset branch 'main' Objects/object.c:2197: _Py_ForgetReference: Assertion failed: invalid object chain Enable tracemalloc to get the memory block allocation traceback object address : 0x7f621294bc50 object refcount : 0 object type : 0x55fbfd156660 object type name: tuple object repr : <refcnt 0 at 0x7f621294bc50> Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed Python runtime state: initialized Thread 0x00007f62215276c0 (most recent call first): File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/test__xxsubinterpreters.py", line 58 in run File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/threading.py", line 989 in run File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/threading.py", line 1052 in _bootstrap_inner File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/threading.py", line 1009 in _bootstrap Current thread 0x00007f6222bb5740 (most recent call first): File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/test__xxsubinterpreters.py", line 48 in _wait_for_interp_to_run File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/test__xxsubinterpreters.py", line 66 in _running File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/contextlib.py", line 137 in __enter__ File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/test__xxsubinterpreters.py", line 701 in test_already_running File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/unittest/case.py", line 589 in _callTestMethod File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/unittest/case.py", line 634 in run File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/unittest/case.py", line 690 in __call__ File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/unittest/suite.py", line 122 in run File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/unittest/suite.py", line 84 in __call__ File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/unittest/suite.py", line 122 in run File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/unittest/suite.py", line 84 in __call__ File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/unittest/suite.py", line 122 in run File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/unittest/suite.py", line 84 in __call__ File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/unittest/runner.py", line 240 in run File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/support/__init__.py", line 1115 in _run_suite File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/support/__init__.py", line 1241 in run_unittest File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/libregrtest/runtest.py", line 294 in _test_module File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/libregrtest/runtest.py", line 330 in _runtest_inner2 File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/libregrtest/runtest.py", line 373 in _runtest_inner File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/libregrtest/runtest.py", line 248 in _runtest File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/libregrtest/runtest.py", line 278 in runtest File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/libregrtest/main.py", line 365 in rerun_failed_tests File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/libregrtest/main.py", line 783 in _main File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/libregrtest/main.py", line 738 in main File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/libregrtest/main.py", line 802 in main File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/test/__main__.py", line 2 in <module> File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/runpy.py", line 88 in _run_code File "/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/Lib/runpy.py", line 198 in _run_module_as_main Extension modules: _testcapi, _xxsubinterpreters (total: 2) make: *** [Makefile:2005: buildbottest] Aborted (core dumped) Cannot open file '/buildbot/buildarea/3.x.pablogsal-arch-x86_64/build/test-results.xml' for upload