1. 27 Jan, 2013 9 commits
  2. 26 Jan, 2013 15 commits
  3. 25 Jan, 2013 1 commit
    • Matěj Laitl's avatar
      Fix calling an "except +" cpp function in a nogil function · 6eb0745f
      Matěj Laitl authored
      For a source:
      |cdef extern from "<foo>":
      |    cdef void foo_cpp() nogil except +
      |
      |cdef call_foo() nogil:
      |    foo_cpp()
      
      We used to generate something like this to actually call foo_cpp() in call_foo():
      |try{foo_cpp();}
      |catch(...) {
      |  Py_BLOCK_THREADS; __Pyx_CppExn2PyErr(); Py_UNBLOCK_THREADS
      |  `code.error_goto(self.pos))`
      |}
      
      where Py_BLOCK_THREADS expands to "PyEval_RestoreThread(_save);".
      __Pyx_CppExn2PyErr() (and alternatives, see SimpleCallNode) calls CPython A API
      methods so it needs to be guarded in a nogil environment.
      
      One problem is that "PyThreadState *_save" is only declared by "with nogil:"
      block, so a .cpp file that doesn't compile is generated for the above code.
      
      However, I think the real issue is that Py_(UN)BLOCK_THREADS is inappropriate
      here, as it isn't allowed to be called recursively and is valid only directly
      in a Py_BEGIN_ALLOW_THREADS ... Py_END_ALLOW_THREADS. IMO PyGILState_Ensure()
      and PyGILState_Release() (through `code.put_ensure_gil()` and a friend) is the
      correct thing to call here as it is allowed to be called recursively and
      actually ensures the current thread can call CPython C API.
      
      This patch does exactly this (and it breaks the generated code to multiple
      lines as it would be way too long otherwise), plus it extends the
      cpp_exceptions_nogil.pyx test with above example that doesn't compile with this
      fix not applied.
      
      Note that we explicitly pass declare_gilstate=True to put_ensure_gil(), as
      PyGILState_Ensure() docs state that recursive calls to it must not share the
      PyGILState_STATE. C++-style declaring a variable inside a block should be
      no-problem here, as try{} .. catch() is obviously valid only in a C++ code.
      
      --HG--
      extra : transplant_source : %AA%F3%BDk%FE%F9%01%7F%8C%A4n%5E%DA4%97%A5%D9%AF%D60
      6eb0745f
  4. 26 Jan, 2013 1 commit
  5. 25 Jan, 2013 2 commits
    • Matěj Laitl's avatar
      Fix calling an "except +" cpp function in a nogil function · 6795a2ba
      Matěj Laitl authored
      For a source:
      |cdef extern from "<foo>":
      |    cdef void foo_cpp() nogil except +
      |
      |cdef call_foo() nogil:
      |    foo_cpp()
      
      We used to generate something like this to actually call foo_cpp() in call_foo():
      |try{foo_cpp();}
      |catch(...) {
      |  Py_BLOCK_THREADS; __Pyx_CppExn2PyErr(); Py_UNBLOCK_THREADS
      |  `code.error_goto(self.pos))`
      |}
      
      where Py_BLOCK_THREADS expands to "PyEval_RestoreThread(_save);".
      __Pyx_CppExn2PyErr() (and alternatives, see SimpleCallNode) calls CPython A API
      methods so it needs to be guarded in a nogil environment.
      
      One problem is that "PyThreadState *_save" is only declared by "with nogil:"
      block, so a .cpp file that doesn't compile is generated for the above code.
      
      However, I think the real issue is that Py_(UN)BLOCK_THREADS is inappropriate
      here, as it isn't allowed to be called recursively and is valid only directly
      in a Py_BEGIN_ALLOW_THREADS ... Py_END_ALLOW_THREADS. IMO PyGILState_Ensure()
      and PyGILState_Release() (through `code.put_ensure_gil()` and a friend) is the
      correct thing to call here as it is allowed to be called recursively and
      actually ensures the current thread can call CPython C API.
      
      This patch does exactly this (and it breaks the generated code to multiple
      lines as it would be way too long otherwise), plus it extends the
      cpp_exceptions_nogil.pyx test with above example that doesn't compile with this
      fix not applied.
      
      Note that we explicitly pass declare_gilstate=True to put_ensure_gil(), as
      PyGILState_Ensure() docs state that recursive calls to it must not share the
      PyGILState_STATE. C++-style declaring a variable inside a block should be
      no-problem here, as try{} .. catch() is obviously valid only in a C++ code.
      6795a2ba
    • Robert Bradshaw's avatar
      Merge pull request #177 from strohel/propagate-error-memoryview · 8164bb31
      Robert Bradshaw authored
      Fix error propagation from memoryview-returning functions
      8164bb31
  6. 24 Jan, 2013 2 commits
    • Matěj Laitl's avatar
      Fix error propagation from memoryview-returning functions · fb8eba69
      Matěj Laitl authored
      A code like
      |cdef double[:] foo():
      |    raise AttributeError('dummy')
      
      generated C code like this:
      |  __pyx_L1_error:;
      |  (...)
      |  __pyx_r.memview = NULL;
      |  __Pyx_AddTraceback("view_return_errors.foo", __pyx_clineno, __pyx_lineno, __pyx_filename);
      |  __pyx_L0:;
      |  if (unlikely(!__pyx_r.memview)) {
      |    PyErr_SetString(PyExc_TypeError,"Memoryview return value is not initialized");
      |  }
      |  __Pyx_RefNannyFinishContext();
      |  return __pyx_r;
      |}
      
      Which is incorrect in error case, because we set __pyx_r.memview to NULL and
      then we test it, so that the PyErr_SetString() is always called in the error
      case, which swallows previously-set error. (it also swallowed the traceback,
      which I don't understand)
      
      A fix is to jump to return_from_error_cleanup label in case return type is
      memoryview, as it is currently done for the case when buffers are present.
      
      A testcase that fauils w/out this fix applied is included, too.
      
      v2: fix test under Python 3 by not using StandardError
      
      --HG--
      extra : transplant_source : G%B5%99Og%D1%81%25k%8F%1F%7B%02V%3E%B9%A4y%FF%EA
      fb8eba69
    • Matěj Laitl's avatar
      Fix error propagation from memoryview-returning functions · ec2c8d97
      Matěj Laitl authored
      A code like
      |cdef double[:] foo():
      |    raise AttributeError('dummy')
      
      generated C code like this:
      |  __pyx_L1_error:;
      |  (...)
      |  __pyx_r.memview = NULL;
      |  __Pyx_AddTraceback("view_return_errors.foo", __pyx_clineno, __pyx_lineno, __pyx_filename);
      |  __pyx_L0:;
      |  if (unlikely(!__pyx_r.memview)) {
      |    PyErr_SetString(PyExc_TypeError,"Memoryview return value is not initialized");
      |  }
      |  __Pyx_RefNannyFinishContext();
      |  return __pyx_r;
      |}
      
      Which is incorrect in error case, because we set __pyx_r.memview to NULL and
      then we test it, so that the PyErr_SetString() is always called in the error
      case, which swallows previously-set error. (it also swallowed the traceback,
      which I don't understand)
      
      A fix is to jump to return_from_error_cleanup label in case return type is
      memoryview, as it is currently done for the case when buffers are present.
      
      A testcase that fauils w/out this fix applied is included, too.
      
      v2: fix test under Python 3 by not using StandardError
      ec2c8d97
  7. 22 Jan, 2013 2 commits
    • Matěj Laitl's avatar
      Add test for memoryview of extension type · de905040
      Matěj Laitl authored
      A test for a bug fixed in commit 478b939a.
      
      v2: add commit link above
      v3: # tag: instead of # tags:, drop cpp tag as it means something different
          that I originally thought
      
      There was a bug that produced C code where gcc emitted warnings:
      extension_type_memoryview.c: In function ‘__pyx_pf_25extension_type_memoryview_test_getitem’:
      extension_type_memoryview.c:1468:15: warning: assignment from incompatible pointer type
      extension_type_memoryview.c: In function ‘__pyx_pf_25extension_type_memoryview_2test_getitem_typed’:
      extension_type_memoryview.c:1565:15: warning: assignment from incompatible pointer type
      extension_type_memoryview.c:1568:18: warning: assignment from incompatible pointer type
      
      And g++ failed with errors:
      extension_type_memoryview.c: In function ‘PyObject* __pyx_pf_25extension_type_memoryview_test_getitem(PyObject*)’:
      extension_type_memoryview.c:1468:213: error: cannot convert ‘__pyx_obj_25extension_type_memoryview_ExtensionType*’ to ‘PyObject*’ in assignment
      extension_type_memoryview.c: In function ‘PyObject* __pyx_pf_25extension_type_memoryview_2test_getitem_typed(PyObject*)’:
      extension_type_memoryview.c:1565:213: error: cannot convert ‘__pyx_obj_25extension_type_memoryview_ExtensionType*’ to ‘PyObject*’ in assignment
      extension_type_memoryview.c:1568:20: error: cannot convert ‘PyObject*’ to ‘__pyx_obj_25extension_type_memoryview_ExtensionType*’ in assignment
      de905040
    • Robert Bradshaw's avatar
      Merge pull request #176 from larsmans/fix-indent · d06c8958
      Robert Bradshaw authored
      fix indentation error in userguide
      d06c8958
  8. 21 Jan, 2013 8 commits