move_median MEMORY_ERR without return — segfault under OOM

[devdanzin]

When mm_new_nan (float path) or mm_new (int path) returns NULL under OOM, the MEMORY_ERR macro sets a MemoryError but does not return. Execution falls through to BN_BEGIN_ALLOW_THREADS (releasing the GIL with a pending exception), then dereferences the NULL mm pointer inside the computation loop — segfault.

File(s): move_template.c:568-570 (float), move_template.c:604-606 (int)

Reproducer:

import bottleneck as bn
import numpy as np
import _testcapi

arr = np.array([1.0, 2.0, 3.0, 4.0, 5.0])
_testcapi.set_nomemory(1, 0)  # Fail all allocations
try:
    bn.move_median(arr, window=3)
    _testcapi.remove_mem_hooks()
except MemoryError:
    _testcapi.remove_mem_hooks()
# Segmentation fault (core dumped)

Crashes at n=1 because mm_new_nan uses raw malloc (hooked by _testcapi.set_nomemory).

Suggested fix: Add return NULL; after each MEMORY_ERR. Also free y from the INIT macro:

// Before:
if (mm == NULL) {
    MEMORY_ERR("Could not allocate memory for move_median");
}

// After:
if (mm == NULL) {
    MEMORY_ERR("Could not allocate memory for move_median");
    Py_DECREF(y);
    return NULL;
}

See #518 for the complete report.

Found using cext-review-toolkit.

[neutrinoceros]

where is _testcapi defined ? It seems necessary to reproduce reliably here

perharps more importantly, I don't get this

When mm_new_nan (float path) or mm_new (int path) returns NULL under OOM, the MEMORY_ERR macro sets a MemoryError but does not return

neither mm_new_nan nor mm_new seem to rely on MEMORY_ERR as far as I can tell. Am I missing anything ?

[devdanzin]

_testcapi is a CPython module (Modules/_testcapimodule.c). _testcapi.set_nomemory(n, 0) makes the first n allocations succeed, then returns NULL every one after that, allowing us to simulate OOM situations.

Regarding mm_new_nan / MEMORY_ERR, the wording was unclear, sorry about that. The bug is in move_template.c, not in mm_new_nan itself. Here's how Claude describes the flow:

1. mm_new_nan(window, min_count) is called at line 562 and returns NULL (OOM)
2. The NULL check at line 568 runs MEMORY_ERR(...) which expands to PyErr_SetString(PyExc_MemoryError, ...) — this sets the exception but does not return
3. Execution falls through to BN_BEGIN_ALLOW_THREADS at line 571 (releasing the GIL with a pending exception)
4. The loop body dereferences mm (which is NULL) → segfault

The fix is adding Py_DECREF(y); return NULL; after the MEMORY_ERR call, so the function actually returns instead of falling through.

[neutrinoceros]

gotcha. I finally took a minute to understand why I couldn't import _testcapi; I was using uv-distributed python interpreters, which explicitly strip this module. I can get it running with pyenv.

[neutrinoceros]

Now the question becomes how do I transform this into a normal test ?
Running the reproducer as is just ends like a successful process with no stderr ouptut.
On the other hand if I remove the try clause and cleanups as

import bottleneck as bn
import numpy as np
import _testcapi

arr = np.array([1.0, 2.0, 3.0, 4.0, 5.0])
_testcapi.set_nomemory(1, 0)  # Fail all allocations
bn.move_median(arr, window=3)

I get the following stderr, which I assume is what _testcapi.set_nomemory(1, 0) prints instead of a real segfault

object address  : 0x100785f00
object refcount : 3
object type     : 0x10129a720
object type name: MemoryError
object repr     : 
lost sys.stderr
object address  : 0x100785ea0
object refcount : 3
object type     : 0x10129a720
object type name: MemoryError
object repr     : 
lost sys.stderr

[devdanzin]

I think it might be necessary to depend on printing some success message when the MemoryError is caught, because the reproducer just catches it and allows the script to terminate normally. I can test this approach later tonight, if you don't solve it before then.

[neutrinoceros]

I couldn't crack it with what little time I had on my hands. I'll let you have a go at it for now. Thank you !