gh-87804: Fix the refleak in error handling of _pystatvfs_fromstructstatfs

[sobolevn]

I messed up macro expansion rules, sorry!

It was:

do { if (PyLong_FromLong((long) st.f_iosize) == ((void*)0)) { Py_DECREF("./Modules/posixmodule.c", 12928, ((PyObject*)((v)))); return ((void*)0); } PyStructSequence_SetItem(v, 0, PyLong_FromLong((long) st.f_iosize)); } while (0);

It is now:

do { PyObject *obj = (PyLong_FromLong((long) st.f_iosize)); if (obj == ((void*)0) || PyErr_Occurred()) { Py_DECREF("./Modules/posixmodule.c", 12929, ((PyObject*)((v)))); return ((void*)0); } PyStructSequence_SetItem(v, (0), obj); } while (0);

• Issue: MacOS os.statvfs() has rollover for >4TB disks at each 4TB (32bit counter overflow?) #87804

[sobolevn]

!buildbot refleak

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit dd821c2 🤖

The command will test the builders whose names match following regular expression: refleak

The builders matched are:

• s390x RHEL8 Refleaks PR
• AMD64 Windows11 Refleaks PR
• AMD64 Fedora Rawhide Refleaks PR
• aarch64 Fedora Stable Refleaks PR
• AMD64 Ubuntu NoGIL Refleaks PR
• ARM64 MacOS M1 Refleaks NoGIL PR
• aarch64 CentOS9 Refleaks PR
• aarch64 RHEL8 Refleaks PR
• s390x RHEL7 Refleaks PR
• PPC64LE Fedora Rawhide Refleaks PR
• PPC64LE RHEL7 Refleaks PR
• PPC64LE Fedora Stable Refleaks PR
• PPC64LE RHEL8 Refleaks PR
• AMD64 RHEL7 Refleaks PR
• s390x Fedora Refleaks PR
• s390x Fedora Rawhide Refleaks PR
• AMD64 Fedora Stable Refleaks PR
• AMD64 CentOS9 Refleaks PR
• AMD64 RHEL8 Refleaks PR
• PPC64LE CentOS9 Refleaks PR
• aarch64 Fedora Rawhide Refleaks PR

[encukou]

Good catch!
Could you also name the macro parameters in ALL_CAPS, see https://peps.python.org/pep-0007/#naming-conventions? This convention makes similar issues easier to spot.

[encukou]

Thanks for the quick fix!

[vstinner]

LGTM. It would be even better with argument name longer than 1 character :-)

[sobolevn]

Once again.

!buildbot refleak

[vstinner]

Once again.

I suggest you to give up on buildbots and just merge your PR if you can prove to yourself that your fix works as expected:

• tests leak before
• tests no longer leak after your change

For example, run ./python.exe -m test bisect test_posix -R 3:3 on macOS. I cannot, I don't have access to macOS.

[sobolevn]

I've done manual testing.

Before:

» ./python.exe -m test -R 3:3 test_os test_pickle test_pickletools test_posix test_shutil
Using random seed: 690889334
0:00:00 load avg: 4.23 Run 5 tests sequentially
0:00:00 load avg: 4.23 [1/5] test_os
beginning 6 repetitions
123456
/Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=8886) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
./Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=8886) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
./Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=8886) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
./Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=8886) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
./Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=8886) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
./Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=8886) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
.
test_os leaked [36, 36, 36] references, sum=108
test_os leaked [36, 36, 36] memory blocks, sum=108
0:00:08 load avg: 3.96 [2/5/1] test_pickle -- test_os failed (reference leak)
beginning 6 repetitions
123456
......
test_pickle leaked [272, 272, 272] references, sum=816
test_pickle leaked [270, 270, 270] memory blocks, sum=810
0:00:45 load avg: 3.27 [3/5/2] test_pickletools -- test_pickle failed (reference leak) in 37.4 sec
beginning 6 repetitions
123456
......
0:00:49 load avg: 3.25 [4/5/2] test_posix
beginning 6 repetitions
123456
......
test_posix leaked [27, 27, 27] references, sum=81
test_posix leaked [27, 27, 27] memory blocks, sum=81
0:00:57 load avg: 3.15 [5/5/3] test_shutil -- test_posix failed (reference leak)
beginning 6 repetitions
123456
......
test_shutil leaked [18, 18, 18] references, sum=54
test_shutil leaked [18, 18, 18] memory blocks, sum=54
test_shutil failed (reference leak)

== Tests result: FAILURE ==

4 tests failed:
    test_os test_pickle test_posix test_shutil

1 test OK.

Total duration: 1 min 1 sec
Total tests: run=1,642 skipped=203
Total test files: run=5/5 failed=4
Result: FAILURE

After:

» ./python.exe -m test -R 3:3 test_os test_pickle test_pickletools test_posix test_shutil
Using random seed: 1906848956
0:00:00 load avg: 1.70 Run 5 tests sequentially
0:00:00 load avg: 1.70 [1/5] test_os
beginning 6 repetitions
123456
/Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=667) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
./Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=667) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
./Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=667) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
./Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=667) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
./Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=667) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
./Users/sobolev/Desktop/cpython2/Lib/os.py:861: DeprecationWarning: This process (pid=667) is multi-threaded, use of fork() may lead to deadlocks in the child.
  pid = fork()
.
0:00:08 load avg: 1.98 [2/5] test_pickle
beginning 6 repetitions
123456
......
0:00:46 load avg: 2.45 [3/5] test_pickletools -- test_pickle passed in 38.2 sec
beginning 6 repetitions
123456
......
0:00:50 load avg: 2.33 [4/5] test_posix
beginning 6 repetitions
123456
......
0:00:58 load avg: 2.20 [5/5] test_shutil
beginning 6 repetitions
123456
......

== Tests result: SUCCESS ==

All 5 tests OK.

Total duration: 1 min 2 sec
Total tests: run=1,642 skipped=203
Total test files: run=5/5
Result: SUCCESS

No refleaks anymore 🎉

I will wait for the CI to get green and merge. Thanks everyone for their feedback!

[vstinner]

Thanks for the fix.