BUG: flaky test under pytest-run-parallel

[ngoldbaum]

==================================== ERRORS ====================================
___________ ERROR at call of test_real_imag_ufunc_minimal[imag-imag] ___________

ufunc = <ufunc 'imag'>, attr = 'imag'

    @pytest.mark.parametrize("ufunc,attr", [
        (np._core.umath.real, "real"), (np._core.umath.imag, "imag")])
    def test_real_imag_ufunc_minimal(ufunc, attr):
        with pytest.raises(TypeError):
            ufunc(np.array([1, 2, 3]))  # non-complex or object raises
    
        arr = np.array([1 + 2j, 3 + 4j])
>       res = ufunc(arr)
              ^^^^^^^^^^
E       RecursionError: Stack overflow (used 16364 kB) in __subclasscheck__

arr        = array([1.+2.j, 3.+4.j])
attr       = 'imag'
ufunc      = <ufunc 'imag'>

As seen here: https://github.com/numpy/numpy/actions/runs/23796435549/job/69344855092?pr=31108#step:9:47471

Almost certainly fallout from merging #30984. I'm going to try to reproduce this locally and understand what's happening.

Ping @seberg @kumaraditya303

[ngoldbaum]

I can reproduce this on my Mac if I do this:

$ while spin test --no-build "numpy/_core/tests/test_arrayobject.py::test_real_imag_ufunc_minimal" -- --parallel-threads=100 -sv; do; done

It crashes after about 10 loops. It needs to be a "fresh" interpreter (using pytest-run-parallel's --forever option doesn't trigger the issue) so this has something to do with cache initialization, which I guess makes sense.

[ngoldbaum]

@seberg while we work on the issues raised in #31083, would you be OK with reverting #30984? We could xfail it as well but since the failure isn't deterministic, it would be the sort of xfail we might forget to remove afterwards. I'm not comfortable with this sort of crash being possible now. It's also not great that CI is hitting it regularly.

[kumaraditya303]

I think the issue is that PyUFunc_AddLoop is called dynamically and there is no serialization or locking so multiple threads could end up adding multiple loops or overwrite loops added by other threads.

[ngoldbaum]

FWIW here's the first few frames of the stack trace on the crashing thread, everything after is the same two frames over and over:

(lldb) bt 100
* thread #4, name = 'Thread-46003 (closure)', stop reason = breakpoint 1.1
  * frame #0: 0x0000000100ae0430 libpython3.14t.dylib`_Py_CheckRecursiveCall(tstate=0x000000012600f800, where=" in comparison") at ceval.c:619:21
    frame #1: 0x00000001009c97b8 libpython3.14t.dylib`_Py_EnterRecursiveCallTstate(tstate=0x000000012600f800, where=" in comparison") at pycore_ceval.h:224:41
    frame #2: 0x00000001009ca3ec libpython3.14t.dylib`PyObject_RichCompare(v=0x0000000102219a18, w=0x0000043b90467800, op=2) at object.c:1106:9
    frame #3: 0x00000001009ca2f4 libpython3.14t.dylib`PyObject_RichCompareBool(v=0x0000000102219a18, w=0x0000043b90467800, op=2) at object.c:1131:11
    frame #4: 0x00000001009ff230 libpython3.14t.dylib`tuple_richcompare(v=0x0000043b92175280, w=0x0000000c0c1d1b00, op=2) at tupleobject.c:687:17
    frame #5: 0x00000001009ca590 libpython3.14t.dylib`do_richcompare(tstate=0x000000012600f800, v=0x0000043b92175280, w=0x0000000c0c1d1b00, op=2) at object.c:1060:15
    frame #6: 0x00000001009ca410 libpython3.14t.dylib`PyObject_RichCompare(v=0x0000043b92175280, w=0x0000000c0c1d1b00, op=2) at object.c:1109:21
    frame #7: 0x00000001009ca2f4 libpython3.14t.dylib`PyObject_RichCompareBool(v=0x0000043b92175280, w=0x0000000c0c1d1b00, op=2) at object.c:1131:11
    frame #8: 0x0000000102083d84 _multiarray_umath.cpython-314t-darwin.so`PyUFunc_AddLoop(ufunc=0x0000043b92100f00, info=0x0000000c0c1d1b40, ignore_duplicate=1) at dispatching.cpp:130:19 [opt]
    frame #9: 0x00000001020a5538 _multiarray_umath.cpython-314t-darwin.so`int real_imag_promoter<&NPY_DType_Slots::imag_meth>(ufunc=0x0000043b92100f00, op_dtypes=<unavailable>, signature=<unavailable>, new_op_dtypes=0x0000000171e244e0) at real_imag_ufuncs.cpp:220:15 [opt]
    frame #10: 0x00000001020850b0 _multiarray_umath.cpython-314t-darwin.so`call_promoter_and_recurse(ufunc=0x0000043b92100f00, info=0x0000043b921752c0, op_dtypes=0x0000000171e249b0, signature=0x0000000172e0f0e0, operands=0x0000000172e0f0f0) at dispatching.cpp:616:31 [opt]
    frame #11: 0x0000000102085088 _multiarray_umath.cpython-314t-darwin.so`promote_and_get_info_and_ufuncimpl(ufunc=<unavailable>, ops=<unavailable>, signature=<unavailable>, op_dtypes=<unavailable>, legacy_promotion_is_possible=<unavailable>) at dispatching.cpp:924:16 [opt]
    frame #12: 0x0000000102085448 _multiarray_umath.cpython-314t-darwin.so`call_promoter_and_recurse(ufunc=0x0000043b92100f00, info=<unavailable>, op_dtypes=0x0000000171e24e80, signature=0x0000000172e0f0e0, operands=0x0000000172e0f0f0) at dispatching.cpp:660:21 [opt] [inlined]
    frame #13: 0x00000001020853fc _multiarray_umath.cpython-314t-darwin.so`promote_and_get_info_and_ufuncimpl(ufunc=<unavailable>, ops=<unavailable>, signature=<unavailable>, op_dtypes=<unavailable>, legacy_promotion_is_possible=<unavailable>) at dispatching.cpp:924:16 [opt]
    frame #14: 0x0000000102085448 _multiarray_umath.cpython-314t-darwin.so`call_promoter_and_recurse(ufunc=0x0000043b92100f00, info=<unavailable>, op_dtypes=0x0000000171e25350, signature=0x0000000172e0f0e0, operands=0x0000000172e0f0f0) at dispatching.cpp:660:21 [opt] [inlined]
    frame #15: 0x00000001020853fc _multiarray_umath.cpython-314t-darwin.so`promote_and_get_info_and_ufuncimpl(ufunc=<unavailable>, ops=<unavailable>, signature=<unavailable>, op_dtypes=<unavailable>, legacy_promotion_is_possible=<unavailable>) at dispatching.cpp:924:16 [opt]
    frame #16: 0x0000000102085448 _multiarray_umath.cpython-314t-darwin.so`call_promoter_and_recurse(ufunc=0x0000043b92100f00, info=<unavailable>, op_dtypes=0x0000000171e25820, signature=0x0000000172e0f0e0, operands=0x0000000172e0f0f0) at dispatching.cpp:660:21 [opt] [inlined]
    frame #17: 0x00000001020853fc _multiarray_umath.cpython-314t-darwin.so`promote_and_get_info_and_ufuncimpl(ufunc=<unavailable>, ops=<unavailable>, signature=<unavailable>, op_dtypes=<unavailable>, legacy_promotion_is_possible=<unavailable>) at dispatching.cpp:924:16 [opt]

[seberg]

Hmmmm, yeah, I guess the loop adding needs a lock in either case maybe? I would prefer not reverting and just adding that lock.
I do care quite a lot about fixing that feature. Now we could remove the ufunc part of it (i.e. the object fix), but that would be a hell of a lot harder than adding the right locking.

EDIT: To be clear, this adds the loop dynamically during promotion. And I think just locking everything during promotion may make sense.
But I expect the issue here is unrelated to that (unless there is a bug in the cache), but rather that adding the loop itself isn't fully thread safe.

[kumaraditya303]

Hmmmm, yeah, I guess the loop adding needs a lock in either case maybe? I would prefer not reverting and just adding that lock.
I do care quite a lot about fixing that feature. Now we could remove the ufunc part of it (i.e. the object fix), but that would be a hell of a lot harder than adding the right locking.

The code in question calls a bunch of CPython C-APIs and I think it is re-entrant so you cannot use a simple mutex, you would have to use a critical section on either an object or on a per-ufunc PyMutex.

[seberg]

I don't think adding a loop should be re-entrant (or at least only in pretty theoretical ways) or am I missing something? Promotion could be more reasonably re-entrant (but IIRC that used to ignore previously, and I can see ignoring it. Except for promotion triggering adding a loop, I guess -- which might be re-entrant if it used the same lock.).

A per-ufunc mutex isn't problematic as such, though? We used to have a per-ufunc shared-mutex until very recently even a re-entrant mutex is probably not a big issue if that is required?
(Critical sections safety to me is very unclear unless its content uses no Python API at all? -- it feels a bit like "maybe as safe as the GIL". But the GIL is at best in practice safe enough in cases such as here?)

(In the end I assume we can say that contention on the mutex is meaningless, because the cache lookup fast-path is the only relevant thing.)

[ngoldbaum]

@seberg is there any chance you'd be available next Wednesday at 4:30 PM UTC for a video call to talk about this? That time works best for Kumar, the NumPy community meetings are too late for him.

Thursday and Friday at the same time also work.