cython/cython

Crash when bounds of prange() are Python objects

Open

#2,504 opened on Jul 18, 2018

 (2 comments) (0 reactions) (0 assignees)Python (1,517 forks)batch import
Code Generationdefectgood first issuehelp wanted

Repository metrics

Stars
 (8,663 stars)
PR merge metrics
 (Avg merge 45d 12h) (52 merged PRs in 30d)

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