On Sun, Jun 20, 2010 at 8:49 PM, Stefan Behnel <[email protected]> wrote: > [taking this to cython-dev, as it's worth going into the archive] > > Haoyu Bai, 20.06.2010 14:22: >>> Haoyu Bai, 20.06.2010 04:48: >>>> compiling (cpp) and running knuth_man_or_boy_test ... Doctest: >>>> knuth_man_or_boy_test.__test__.a (line 44) ... python: >>>> Modules/gcmodule.c:276: visit_decref: Assertion `gc->gc.gc_refs != 0' >>>> failed. >>>> >>>> This seems related to refcounting problem due to my work on nonlocal. >>>> I attempted to fix it but no success yet. In knuth_man_or_boy_test, >>>> the assertion failed on a binding_PyCFunction object (the function >>>> object b). >> >> >> I solved the problem. This is due to GC collecting may start inside a >> refnanny Pyx_INCREF or Pyx_DECREF. > > Sure, DECREF can run arbitrary code. INCREF can't, though. Even in PyDebug > builds, it's pretty restricted. > >
However, with refnanny, INCREF may also run arbitrary code (at least trigger GC). That may cause strange things. We should be aware. >> Thus a inconsistent state could be exposed to the GC. >> >> For example I was writing code like this: >> >> scope->v1 = arg1; >> scope->v2 = arg2; >> scope->v3 = arg3; >> Pyx_INCREF(scope->v1); Pyx_GIVEREF(scope->v1); >> Pyx_INCREF(scope->v2); Pyx_GIVEREF(scope->v2); >> Pyx_INCREF(scope->v3); Pyx_GIVEREF(scope->v3); > > Don't ever do that! > > The correct way to do this is to do the INCREF first, before the assignment, > i.e. in this order: > > Pyx_INCREF(arg1); > Pyx_DECREF(scope->v1); (if required) > scope->v1 = arg1; > Pyx_GIVEREF(scope->v1); (or GIVEREF(arg1)) > > Actually, it would be even better to do this instead: > > Pyx_INCREF(arg1); > temp = scope->v1; > scope->v1 = arg1; > Pyx_DECREF(temp); > Pyx_GIVEREF(arg1); > > as DECREF(scope->v1) can run arbitrary code that may involve traversing the > references stored in "scope", including the reference in "scope->v1". > > Read up on the Py_CLEAR() macro. > > Stefan > Thanks for the comments. I fixed my branch with the correct way. Now the build status on Hudson all turns blue or yellow. Indeed, there are some other places that we should use Py_CLEAR() insted of DECREF. I'll try to go through and fix them. -- Haoyu BAI School of Computing, National University of Singapore. _______________________________________________ Cython-dev mailing list [email protected] http://codespeak.net/mailman/listinfo/cython-dev
