[ python-Bugs-840829 ] weakref callbacks and gc corrupt memory

[SourceForge.net]

Bugs item #840829, was opened at 2003-11-12 12:05
Message generated for change (Comment added) made by tim_one
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=840829&group_id=5470

Category: Python Interpreter Core
Group: Python 2.3
Status: Open
Resolution: None
Priority: 9
Submitted By: Thomas Heller (theller)
Assigned to: Thomas Heller (theller)
Summary: weakref callbacks and gc corrupt memory

Initial Comment:
This program crashes the Python interpreter in a debug
build, and probably corrupts memory in release build
(on Windows, at least):

"""
import weakref
import gc

_boundMethods = weakref.WeakKeyDictionary()

def safeRef(object):
    selfkey = object.im_self
    funckey = object.im_func
    _boundMethods[selfkey] = weakref.WeakKeyDictionary()
    _boundMethods[selfkey][funckey] =
BoundMethodWeakref(object)

class BoundMethodWeakref:
    def __init__(self, boundMethod):
        def remove(object):
            gc.collect()
        self.weakSelf =
weakref.ref(boundMethod.im_self, remove)

class X(object):
    def test(self):
        pass

def test():
    print "A"
    safeRef(X().test)
    print "B"

if __name__ == "__main__":
    test()
"""

See also these messages:

http://mail.python.org/pipermail/python-dev/2003-November/040189.html

http://mail.python.org/pipermail/python-dev/2003-November/040191.html


----------------------------------------------------------------------

>Comment By: Tim Peters (tim_one)
Date: 2003-11-13 17:55

Message:
Logged In: YES 
user_id=31435

What I hope is the last fix for this class of bug has been 
checked in on release23-maint too, so feel free to use that 
for testing instead.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2003-11-13 17:23

Message:
Logged In: YES 
user_id=31435

Good!  I hoped it would fix it, but it's not the best way to fix 
it.

I've checked in a better way, so please test against the 
current CVS trunk.  I'll backport that to the 2.3 maint branch 
too, but later today.

----------------------------------------------------------------------

Comment By: Thomas Heller (theller)
Date: 2003-11-13 16:13

Message:
Logged In: YES 
user_id=11105

Applied your patch, very cool!  This problem doesn't occur
any more, it seems.  But this was only after a short test
with my application.
I will patch the 2.3 branch and test more extensively with
this one tomorrow.

----------------------------------------------------------------------

Comment By: Thomas Heller (theller)
Date: 2003-11-13 16:06

Message:
Logged In: YES 
user_id=11105

> When you say "what helped for me was ...", do you mean
> that cured *all* the problems you're seeing, or just that "it
> helped", in the ordinary sense that it cured more of the
> problems, but not all of the problems.

Well, it seemed to help "completely", I never observed the
crash with this workaround.

For the patch to subtype_dealloc, I will try your suggestion
and report.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2003-11-13 08:56

Message:
Logged In: YES 
user_id=31435

Thanks!  There's more than one bug here, of course.

When you say "what helped for me was ...", do you mean 
that cured *all* the problems you're seeing, or just that "it 
helped", in the ordinary sense that it cured more of the 
problems, but not all of the problems.

Since I don't have a failing program I can run at this point, I 
can't test any other ideas.  You <wink> can, though:

In typeobject.c's subtype_dealloc, on the trunk, right before 
the comment

/*  Clear slots up to the nearest base with a different 
tp_dealloc */

try inserting this line:

    self->ob_refcnt = 1;

and right after the

	/* Call the base tp_dealloc() */

comment insert

    assert(self->ob_refcnt == 1);
    self->ob_refcnt = 0;

That should cure the more-complicated (than in your original 
report) bug in the new stacktrace.  However, it *may* cause 
some other code to die with an assert, bitching about the 
suddenly non-zero refcnt.  I can't guess whether it will; my 
best guess is that it won't.

----------------------------------------------------------------------

Comment By: Thomas Heller (theller)
Date: 2003-11-13 02:49

Message:
Logged In: YES 
user_id=11105

Thanks for the fast response, Tim, but this doesn't seem to
be the (complete) solution.

The debug build crashes far less than before, but it still
occurs, and in the same way.  I attach a complete MSVC6
stack dump.  The function names you do not know come from
the ctypes extension.

Back to you again :-(

Oh, what helped for me was to disable gc in the weakref
callback functions.  Don't know if that is an option for the
C code...

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2003-11-12 15:47

Message:
Logged In: YES 
user_id=31435

Back to you, Thomas!  Please check your real app against 
current CVS Python.  I checked in a putative fix as

Lib/test/test_weakref.py; new revision: 1.29
Misc/NEWS; new revision: 1.890
Objects/typeobject.c; new revision: 2.251

The new test case in test_weakref.py is very much simpler 
than what you've been running, so I want confirmation that 
the real problem is fixed too before closing this.

Please record what happens here, then assign back to me.

----------------------------------------------------------------------

Comment By: Tim Peters (tim_one)
Date: 2003-11-12 14:54

Message:
Logged In: YES 
user_id=31435

I agree this is a critical bug; assigned to me.

----------------------------------------------------------------------

Comment By: Thomas Heller (theller)
Date: 2003-11-12 12:08

Message:
Logged In: YES 
user_id=11105

Attached the script triggering the error.

----------------------------------------------------------------------

Comment By: Thomas Heller (theller)
Date: 2003-11-12 12:07

Message:
Logged In: YES 
user_id=11105

IMO this is a serious problem because it renders weakref
callbacks basically unusable - gc can occur at any time.
So I raised the priority to 9.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=840829&group_id=5470