bpo-38168: Fix possible refleaks in setint() of mmapmodule.c

[shihai1991]

https://bugs.python.org/issue38168

[matrixise]

Could you reproduce with a test? and via this command?

./python -m test test_XYZ -R 3:3

replace test_XYZ by a test where you test the memory leak.

[shihai1991]

Could you reproduce with a test? and via this command?

./python -m test test_XYZ -R 3:3

replace test_XYZ by a test where you test the memory leak.

I have the same question too, but i found some PR or bpo no regression test against it :(
[1] #8934
[2] https://bugs.python.org/issue27248

[ghost]

In current code, if PyDict_SetItemString() fails (return -1), o will not be released.
It's hard to reproduce a leak, it requires harsh condition.

Another way is to use PyModule_AddIntMacro(), and no longer use setint() in Modules/mmapmodule.c.
IMO PyModule_AddIntMacro()is more suitable, but maybe a bit slower.

[shihai1991]

In current code, if PyDict_SetItemString() failes (return -1), o will not be released.
It's hard to reproduce a leak, it requires harsh condition.

Agree.

Another way is to use PyModule_AddIntMacro(), and no longer use setint() in Modules/mmapmodule.c.
IMO PyModule_AddIntMacro()is more suitable, but maybe a bit slower.

I review the PyModule_AddIntMacro code, looks your are right. But as your said, it MAYBE a bit slower.

[ghost]

I would suggest use "possible" in the title, "Fix possible refleaks..."

[shihai1991]

I would suggest use "possible" in the title, "Fix possible refleaks..."

done :)

[zhangyangyu]

I think for this tiny and clear fix, which also hard to reproduce, it's okay to omit test.

[miss-islington]

Thanks @shihai1991 for the PR, and @zhangyangyu for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

[bedevere-bot]

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

[bedevere-bot]

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

[miss-islington]

Thanks @shihai1991 for the PR, and @zhangyangyu for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7.
🐍🍒⛏🤖

[miss-islington]

Sorry, @shihai1991 and @zhangyangyu, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 56a45142e70a1ccf3233d43cb60c47255252e89a 2.7

[bedevere-bot]

GH-16176 is a backport of this pull request to the 2.7 branch.

[bedevere-bot]

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

Hi! The buildbot AMD64 Windows7 SP1 3.7 has failed when building commit e857fe6.

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/130/builds/1125) and take a look at the build logs.
4. Check if the failure is related to this commit (e857fe6) 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/130/builds/1125

Failed tests:

• test_regrtest

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

== Tests result: FAILURE then SUCCESS ==

382 tests OK.

10 slowest tests:

• test_io: 3 min 5 sec
• test_multiprocessing_spawn: 2 min 38 sec
• test_tools: 2 min 18 sec
• test_mmap: 1 min 48 sec
• test_venv: 1 min 47 sec
• test_tokenize: 1 min 44 sec
• test_concurrent_futures: 1 min 42 sec
• test_lib2to3: 1 min 16 sec
• test_socket: 1 min 3 sec
• test_asyncio: 55 sec 342 ms

34 tests skipped:
test_crypt 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_nis test_openpty
test_ossaudiodev test_pipes test_poll test_posix test_pty test_pwd
test_readline test_resource test_spwd test_syslog
test_threadsignals test_tix test_tk test_ttk_guionly test_wait3
test_wait4 test_xxtestfuzz test_zipfile64

1 re-run test:
test_regrtest

Total duration: 17 min 16 sec

Click to see traceback logs

Traceback (most recent call last):
  File "C:\buildbot.python.org\3.7.kloth-win64\build\lib\test\support\__init__.py", line 310, in _force_run
    return func(*args)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\buildbot.python.org\\3.7.kloth-win64\\build\\build\\test_python_4252\\test_python_worker_3640'


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.7.kloth-win64\build\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\buildbot.python.org\3.7.kloth-win64\build\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "C:\buildbot.python.org\3.7.kloth-win64\build\lib\test\__main__.py", line 2, in <module>
    main()
  File "C:\buildbot.python.org\3.7.kloth-win64\build\lib\test\libregrtest\main.py", line 689, in main


Traceback (most recent call last):
  File "C:\buildbot.python.org\3.7.kloth-win64\build\lib\test\support\__init__.py", line 988, in temp_dir
    yield path
  File "C:\buildbot.python.org\3.7.kloth-win64\build\lib\test\support\__init__.py", line 1040, in temp_cwd
    yield cwd_dir
  File "C:\buildbot.python.org\3.7.kloth-win64\build\lib\test\libregrtest\main.py", line 623, in main
    self._main(tests, kwargs)
  File "C:\buildbot.python.org\3.7.kloth-win64\build\lib\test\libregrtest\main.py", line 684, in _main
    sys.exit(0)
SystemExit: 0