Update of /cvsroot/python/python/dist/src/Modules In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv4662/Modules Modified Files: Tag: release23-maint gcmodule.c Log Message: handle_weakrefs(): Simplification -- there's no need to make a second pass over the unreachable weakrefs-with-callbacks to unreachable objects. Index: gcmodule.c =================================================================== RCS file: /cvsroot/python/python/dist/src/Modules/gcmodule.c,v retrieving revision 2.71.10.3 retrieving revision 2.71.10.4 diff -u -d -r2.71.10.3 -r2.71.10.4 --- gcmodule.c 31 Oct 2004 00:13:06 -0000 2.71.10.3 +++ gcmodule.c 31 Oct 2004 22:27:45 -0000 2.71.10.4 @@ -453,24 +453,19 @@ PyGC_Head *gc; PyObject *op; /* generally FROM_GC(gc) */ PyWeakReference *wr; /* generally a cast of op */ - PyGC_Head wrcb_to_call; /* weakrefs with callbacks to call */ - PyGC_Head wrcb_to_kill; /* weakrefs with callbacks to ignore */ - PyGC_Head *next; int num_freed = 0; gc_list_init(&wrcb_to_call); - gc_list_init(&wrcb_to_kill); /* Clear all weakrefs to the objects in unreachable. If such a weakref * also has a callback, move it into `wrcb_to_call` if the callback - * needs to be invoked, or into `wrcb_to_kill` if the callback should - * be ignored. Note that we cannot invoke any callbacks until all - * weakrefs to unreachable objects are cleared, lest the callback - * resurrect an unreachable object via a still-active weakref. That's - * why the weakrefs with callbacks are moved into different lists -- we - * make another pass over those lists after this pass completes. + * needs to be invoked. Note that we cannot invoke any callbacks until + * all weakrefs to unreachable objects are cleared, lest the callback + * resurrect an unreachable object via a still-active weakref. We + * make another pass over wrcb_to_call, invoking callbacks, after this + * pass completes. */ for (gc = unreachable->gc.gc_next; gc != unreachable; gc = next) { PyWeakReference **wrlist; @@ -488,7 +483,7 @@ /* `op` may have some weakrefs. March over the list, clear * all the weakrefs, and move the weakrefs with callbacks - * into one of the wrcb_to_{call,kill} lists. + * that must be called into wrcb_to_call. */ for (wr = *wrlist; wr != NULL; wr = *wrlist) { PyGC_Head *wrasgc; /* AS_GC(wr) */ @@ -519,7 +514,9 @@ * callback. Then the callback could resurrect insane objects. * * Since the callback is never needed and may be unsafe in this case, - * wr is moved to wrcb_to_kill. + * wr is simply left in the unreachable set. Note that because we + * already called _PyWeakref_ClearRef(wr), its callback will never + * trigger. * * OTOH, if wr isn't part of CT, we should invoke the callback: the * weakref outlived the trash. Note that since wr isn't CT in this @@ -528,62 +525,28 @@ * nothing in CT is reachable from the callback either, so it's hard * to imagine how calling it later could create a problem for us. wr * is moved to wrcb_to_call in this case. - * - * Obscure: why do we move weakrefs with ignored callbacks to a list - * we crawl over later, instead of getting rid of the callback right - * now? It's because the obvious way doesn't work: setting - * wr->wr_callback to NULL requires that we decref the current - * wr->wr_callback. But callbacks are also weakly referenceable, so - * wr->wr_callback may *itself* be referenced by another weakref with - * another callback. The latter callback could get triggered now if - * we decref'ed now, but as explained before it's potentially unsafe to - * invoke any callback before all weakrefs to CT are cleared. */ + if (IS_TENTATIVELY_UNREACHABLE(wr)) + continue; + assert(IS_REACHABLE(wr)); + /* Create a new reference so that wr can't go away * before we can process it again. */ Py_INCREF(wr); - /* Move wr to the appropriate list. */ + /* Move wr to wrcb_to_call, for the next pass. */ wrasgc = AS_GC(wr); - if (wrasgc == next) - next = wrasgc->gc.gc_next; + assert(wrasgc != next); /* wrasgc is reachable, but + next isn't, so they can't + be the same */ gc_list_remove(wrasgc); - gc_list_append(wrasgc, - IS_TENTATIVELY_UNREACHABLE(wr) ? - &wrcb_to_kill : &wrcb_to_call); - wrasgc->gc.gc_refs = GC_REACHABLE; - } - } - - /* Now that all weakrefs to trash have been cleared, it's safe to - * decref the callbacks we decided to ignore. We cannot invoke - * them because they may be able to resurrect unreachable (from - * outside) objects. - */ - while (! gc_list_is_empty(&wrcb_to_kill)) { - gc = wrcb_to_kill.gc.gc_next; - op = FROM_GC(gc); - assert(IS_REACHABLE(op)); - assert(PyWeakref_Check(op)); - assert(((PyWeakReference *)op)->wr_callback != NULL); - - /* Give up the reference we created in the first pass. When - * op's refcount hits 0 (which it may or may not do right now), - * op's tp_dealloc will decref op->wr_callback too. - */ - Py_DECREF(op); - if (wrcb_to_kill.gc.gc_next == gc) { - /* object is still alive -- move it */ - gc_list_remove(gc); - gc_list_append(gc, old); + gc_list_append(wrasgc, &wrcb_to_call); } - else - ++num_freed; } - /* Finally, invoke the callbacks we decided to honor. It's safe to - * invoke them because they cannot reference objects in `unreachable`. + /* Invoke the callbacks we decided to honor. It's safe to invoke them + * because they can't reference unreachable objects. */ while (! gc_list_is_empty(&wrcb_to_call)) { PyObject *temp; @@ -606,7 +569,14 @@ /* Give up the reference we created in the first pass. When * op's refcount hits 0 (which it may or may not do right now), - * op's tp_dealloc will decref op->wr_callback too. + * op's tp_dealloc will decref op->wr_callback too. Note + * that the refcount probably will hit 0 now, and because this + * weakref was reachable to begin with, gc didn't already + * add it to its count of freed objects. Example: a reachable + * weak value dict maps some key to this reachable weakref. + * The callback removes this key->weakref mapping from the + * dict, leaving no other references to the weakref (excepting + * ours). */ Py_DECREF(op); if (wrcb_to_call.gc.gc_next == gc) {