Tracker: Patches

9 DECREF INCREF Problem (probably severe) - ID: 3440738
Last Update: Comment added ( mhammond )

See also bug #1590399

I still have nasty problems with Python objects magically changing / loosing attributes shortly after creation, while using win32ui and heavy threading. Mainly on machines with many Cores/CPUs.

Patch below: I found 2 strongly smelling locations in win32ui, which are probably responsible: where objects are DECREF'ed and possibly INCREF'ed soon after - while "hoping" that they do not disappear. But I guess this is not valid (today). A new Python object somewhere may use this memory meanwhile and then the object may be stolen back and going hybrid ...

(Unfortunately I don't have the MS full version compilers to compile current MFC based stuff. (Or is this possible somehow with VC Express versions?) I could make quick tests of win32ui.pyd (py2.3 and py2.6)


diff -ur --strip _orig/win32assoc.cpp ./win32assoc.cpp
--- _orig/win32assoc.cpp 2009-03-04 11:52:00 +0000
+++ ./win32assoc.cpp 2011-11-21 13:10:42 +0000
@@ -228,11 +228,11 @@
// So set the instance to NULL _before_ we decref it!
PyObject *old = pAssoc->virtualInst;
pAssoc->virtualInst = NULL;
- XDODECREF(old);
if (ob!=Py_None) {
pAssoc->virtualInst = ob;
DOINCREF(ob);
}
+ XDODECREF(old);
RETURN_NONE;
}

diff -ur --strip _orig/win32cmd.cpp ./win32cmd.cpp
--- _orig/win32cmd.cpp 2009-01-08 22:33:00 +0000
+++ ./win32cmd.cpp 2011-11-21 13:10:26 +0000
@@ -208,17 +208,20 @@
RETURN_ERR("The parameter must be a callable object or None");

void *oldMethod = NULL;
- // note I maybe decref, then maybe incref. I assume object wont be destroyed
- // (ie, ref go to zero) between the 2 calls!)
+ // we need incref's early in order to avoid a ref erronously going to zero during DEDECREF
+ if (method!=Py_None) {
+ Py_INCREF(method);
+ Py_INCREF(hookedObject);
+ }
if (pList->Lookup(message, oldMethod)) {
pList->RemoveKey(message);
// oldMethod is returned - don't drop its reference.
DODECREF(hookedObject);
}
if (method!=Py_None) {
- Py_INCREF(method);
+ // already done above: Py_INCREF(method);
pList->SetAt(message,method);
- Py_INCREF(hookedObject);
+ // already done above: Py_INCREF(hookedObject);
}
if (oldMethod)
return (PyObject *)oldMethod;

kxroberto ( kxroberto ) - 2011-11-21 05:54:53 PST

9

Closed

Fixed

Nobody/Anonymous

None

None

Public


Comment ( 1 )

Date: 2012-01-01 17:44:24 PST
Sender: mhammondProject AdminAccepting Donations

Thanks! I think that first patch could also be done by checking the new
object isn't the same as the old, but I did it your way :) The second one
I made simpler - just always incref to add a temp ref and always decref at
the end - I've attached that patch below - please let me know if you think
it will not work correctly for some reason. Checked in as rev
4173:95b4f896b100

diff --git a/Pythonwin/win32cmd.cpp b/Pythonwin/win32cmd.cpp
--- a/Pythonwin/win32cmd.cpp
+++ b/Pythonwin/win32cmd.cpp
@@ -208,8 +208,10 @@
RETURN_ERR("The parameter must be a callable object or
None");

void *oldMethod = NULL;
- // note I maybe decref, then maybe incref. I assume object wont be
destroyed
- // (ie, ref go to zero) between the 2 calls!)
+ // note I maybe decref, then maybe incref. To ensure the object
will
+ // not be destroyed (ie, ref go to zero) between the 2 calls), I
+ // add a temporary reference first.
+ DOINCREF(hookedObject);
if (pList->Lookup(message, oldMethod)) {
pList->RemoveKey(message);
// oldMethod is returned - don't drop its reference.
@@ -220,6 +222,7 @@
pList->SetAt(message,method);
Py_INCREF(hookedObject);
}
+ DODECREF(hookedObject); // remove temp reference added above.
if (oldMethod)
return (PyObject *)oldMethod;
else

Attached File ( 1 )

Filename Description Download
DECREF_INCREF_problem2011.patch Download

Changes ( 6 )

Field Old Value Date By
status_id Open 2012-01-01 17:44:24 PST mhammond
resolution_id None 2012-01-01 17:44:24 PST mhammond
allow_comments 1 2012-01-01 17:44:24 PST mhammond
close_date - 2012-01-01 17:44:24 PST mhammond
priority 5 2011-11-21 05:56:26 PST kxroberto
File Added 428987: DECREF_INCREF_problem2011.patch 2011-11-21 05:54:54 PST kxroberto