classification
Title: add a nomemory_allocator to the _testcapi module
Type: enhancement Stage: resolved
Components: Tests Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: haypo, serhiy.storchaka, xdegaye
Priority: normal Keywords: patch

Created on 2017-06-18 10:21 by xdegaye, last changed 2017-10-23 13:24 by xdegaye. This issue is now closed.

Files
File name Uploaded Description Edit
nomemory_allocator.patch xdegaye, 2017-06-18 10:21
Pull Requests
URL Status Linked Edit
PR 2406 merged xdegaye, 2017-06-26 13:04
PR 4083 merged python-dev, 2017-10-23 12:34
Messages (19)
msg296266 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-06-18 10:21
Add the set_nomemory_allocator() function to _testcapi that sets a no memory allocator.
msg296276 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-18 16:37
I think the allocators must just return NULL, without setting the error.
msg296299 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-18 23:08
Nice. Do you know my https://pypi.python.org/pypi/pyfailmalloc project? It
helped me to identify and fix dozens of code which didn't handle properly
memory allocation failures. Seach for "pyfailmalloc" in the bug tracker
(closed issues) ;-)
msg296318 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-06-19 12:05
Yes, actually I thought first about re-opening issue 19817 instead of opening this new issue.
msg296320 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-06-19 12:15
> I think the allocators must just return NULL, without setting the error.
You are right, thanks.
msg296323 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-19 12:31
The main difference between your change and pyfailmalloc is the ability in pyfailmalloc to only fail in N allocations. If you want to test various code paths, you need to allow N allocations to reach the deep code that you want to test.

My code is something like 100 lines of C code.
https://bitbucket.org/haypo/pyfailmalloc/src/cdaf3609a30b88243d04e79d6074f3b28d9b64e3/failmalloc.c?at=default&fileviewer=file-view-default

IMHO it's small enough to fit directly into _testcapi.

The question is more what do you want to do with it? It was proposed to "include" pyfailmalloc directly in the Python test suite. Maybe add an option to enable it?

I ran the Python test suite with pyfailmalloc in gdb using this script:
https://bitbucket.org/haypo/pyfailmalloc/src/cdaf3609a30b88243d04e79d6074f3b28d9b64e3/debug_cpython.gdb?at=default&fileviewer=file-view-default

./python -m test -F -r -v -x test_tracemalloc

I waited for the next *crash* and ignored all expected tests failing with MemoryError. --forever (-F) is nice to run random tests until you get a crash.
msg296331 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-06-19 13:28
The chain of events following a call to set_nomemory_allocator() is deterministic, this is another difference with pyfailmalloc. Also what happens then after this call is not very close to real life as memory is never freed.

Having the possibility to trigger memory exhaustion in N allocations instead of rightaway would be very useful. Maybe this could be done by including pyfailmalloc in _testcapi with some changes to enable multiple behaviors, among which one of them would be deterministic.
msg296333 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-19 13:32
Your code can be reproduced with pyfailmalloc using N=0 :-)
msg296338 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-06-19 13:50
I am not sure, failmalloc.enable(range) accepts only range > 1, hook_malloc() fails only once instead of permanently and hook_free() does free memory.
msg296339 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-19 13:52
> I am not sure, failmalloc.enable(range) accepts only range > 1, hook_malloc() fails only once instead of permanently and hook_free() does free memory.

All these things can change :-)

> Also what happens then after this call is not very close to real life as memory is never freed.

Why not freeing memory?
msg296340 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-06-19 14:00
Good point. Not freeing the memory made the implementation of set_nomemory_allocator() simpler, no need to call PyMem_GetAllocator(). Forget about this point, sorry :-)
msg296889 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-26 13:15
I prefer your new PR. But is it useful to test N memory allocation failures in a row? The API and the code would be simpler if we would only test a single failure. What do you think?

I know that pyfailmalloc is different, but I'm not sure that pyfailmalloc design is right neither :-)
msg296893 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-06-26 13:34
For the record, while working on the test case of PR 2406, I found by chance that the following script:

# Script start.
import _testcapi
class C(): pass
_testcapi.set_nomemory(0, 5)
C()
# Script end.

fails with:

python: Objects/call.c:89: _PyObject_FastCallDict: Assertion `!PyErr_Occurred()' failed.
Aborted (core dumped)

I will create a new issue when the current issue is closed.
msg296894 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-06-26 13:36
> But is it useful to test N memory allocation failures in a row?

I think it is useful. See my previous post.
msg296900 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-26 13:53
2017-06-26 15:34 GMT+02:00 Xavier de Gaye <report@bugs.python.org>:
> python: Objects/call.c:89: _PyObject_FastCallDict: Assertion `!PyErr_Occurred()' failed.
> Aborted (core dumped)
>
> I will create a new issue when the current issue is closed.

Oh, I'm curious about that one :-)
msg297412 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-06-30 14:23
> Oh, I'm curious about that one :-)

This is issue 30817.
msg297481 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-07-01 12:14
New changeset 85f643023fed3d4e2fb8e399f9ad57f3a65ef237 by xdegaye in branch 'master':
bpo-30695: Add set_nomemory(start, stop) to _testcapi (GH-2406)
https://github.com/python/cpython/commit/85f643023fed3d4e2fb8e399f9ad57f3a65ef237
msg304797 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-10-23 12:31
Test cases in issues #30697 and #30817, back ported to 3.6, need _testcapi.set_nomemory().
msg304802 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2017-10-23 13:05
New changeset aaf6a3dbbdb9754f98d480b468adfcae0f66e3a2 by xdegaye (Miss Islington (bot)) in branch '3.6':
[3.6] bpo-30695: Add set_nomemory(start, stop) to _testcapi (GH-2406) (#4083)
https://github.com/python/cpython/commit/aaf6a3dbbdb9754f98d480b468adfcae0f66e3a2
History
Date User Action Args
2017-10-23 13:24:32xdegayesetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-10-23 13:05:48xdegayesetmessages: + msg304802
2017-10-23 12:34:19python-devsetstage: patch review
pull_requests: + pull_request4053
2017-10-23 12:31:36xdegayesetmessages: + msg304797
versions: + Python 3.6
2017-07-01 12:14:47xdegayesetmessages: + msg297481
2017-06-30 14:23:08xdegayesetmessages: + msg297412
2017-06-26 13:53:53hayposetmessages: + msg296900
2017-06-26 13:36:51xdegayesetmessages: + msg296894
2017-06-26 13:34:29xdegayesetmessages: + msg296893
2017-06-26 13:15:02hayposetmessages: + msg296889
2017-06-26 13:04:39xdegayesetpull_requests: + pull_request2454
2017-06-25 09:10:19xdegayesetpull_requests: - pull_request2415
2017-06-23 20:31:48xdegayesetpull_requests: + pull_request2415
2017-06-23 16:00:23xdegayesetpull_requests: - pull_request2406
2017-06-23 15:51:11xdegayesetpull_requests: + pull_request2406
2017-06-19 14:00:55xdegayesetmessages: + msg296340
2017-06-19 13:52:33hayposetmessages: + msg296339
2017-06-19 13:50:32xdegayesetmessages: + msg296338
2017-06-19 13:32:50hayposetmessages: + msg296333
2017-06-19 13:28:23xdegayesetmessages: + msg296331
2017-06-19 12:31:59hayposetmessages: + msg296323
2017-06-19 12:15:15xdegayesetmessages: + msg296320
2017-06-19 12:05:54xdegayesetmessages: + msg296318
2017-06-18 23:08:20hayposetmessages: + msg296299
2017-06-18 16:37:22serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg296276
2017-06-18 10:21:11xdegayecreate