cython/cython

Crash when bounds of prange() are Python objects

Open

#2,504 opened on Jul 18, 2018

View on GitHub
 (2 comments) (0 reactions) (0 assignees)Python (8,663 stars) (1,517 forks)batch import
Code Generationdefectgood first issuehelp wanted

Description

The following construct crashes:

cdef unsigned long i
cdef double[:] data = ...
for i in prange(len(data), nogil=True):
    ...

The generated C code is as follows:

      #ifdef WITH_THREAD
      PyThreadState *_save;
      Py_UNBLOCK_THREADS
      __Pyx_FastGIL_Remember();
      #endif
      /*try:*/ {
        if (unlikely(!__pyx_v_data.memview)) { __Pyx_RaiseUnboundMemoryviewSliceNogil("data"); __PYX_ERR(0, 24, __pyx_L4_error) }
        __pyx_t_1 = __pyx_memoryview_fromslice(__pyx_v_data, 1, (PyObject *(*)(char *)) __pyx_memview_get_double, (int (*)(char *, PyObject *)) __pyx_memview_set_double, 0);; if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 24, __pyx_L4_error)
        __Pyx_GOTREF(__pyx_t_1);
        __pyx_t_2 = PyObject_Length(__pyx_t_1); if (unlikely(__pyx_t_2 == ((Py_ssize_t)-1))) __PYX_ERR(0, 24, __pyx_L4_error)
        __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;

len() is evaluated after releasing the GIL. The loop bounds should be evaluated before, and the GIL check applied to all parts of the loop.

Also, actually evaluating Python's len() after first converting the memory view to a Python object is stupid. Might also be part of the problem here.

Contributor guide

Tech stack
pythonc
Domain
toolingperformance
Issue type
bug
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
4
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
over 1 week
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
stale
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
clear
Prerequisites
Cython prange semanticsPython GILCython code generationMemoryview slices
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
25
Research direction
Start by analyzing the generated C code snippet for prange with Python objects as bounds. The issue shows that len() is called after GIL release; the fix likely involves ensuring bounds are evaluated before GIL release. Examine Cython's source files in Cython/Compiler/ or Cython/Utility/ for prange code generation logic. Check if there are any existing PRs or comments addressing this. Reproduce the crash in the latest Cython version to confirm it still exists. Consider modifying the codegen to pre evaluate bounds like pyx t 2 before the GIL is released.
Crash when bounds of prange() are Python objects · cython/cython#2504 | Good First Issue