Message22913
Logged In: YES
user_id=31435
> The approach looks sound but I believe you forgot to clear
> the wr_callback pointer. I'm attaching an incremental patch
> (relative to patch3.txt).
Some of the comments in that incremental patch are good
additions, but I don't agree with the code changes. A
weakref object owns a (strong) reference to its wr_callback,
and the first pass gives move_troublemakers its own new
strong reference to each weakref object. Because of the
latter, nothing any callback does can cause any of these
weakrefs to go away, so nothing can happen to cause their
wr_callback slots to become NULL either. The assertion that
wr_callback isn't NULL is appropriate. If you still think it's
possible for wr_callback to become NULL, please concoct a
test showing it happen.
The passes after the first give up move_troublemaker's
references to the weakref objects. Then they'll go away,
when their refcounts hit 0. Sometimes that happens right
away, sometimes not. In either case, it's weakref_dealloc's
job to decrement the refcount on w->wr_callback when w
goes away, and there's no point (that I can see) to
duplicating that logic too inside gcmodule.
Note that the comments before _PyWeakref_ClearRef()
explain that the weakref's tp_dealloc is expected to finish the
job (clear the wr_callback slot) -- that's been true since
_PyWeakref_ClearRef() was first introduced. |
|
Date |
User |
Action |
Args |
2007-08-23 14:27:07 | admin | link | issue1055820 messages |
2007-08-23 14:27:07 | admin | create | |
|