search

HaveF / shedskin

Automatically exported from code.google.com/p/shedskin
0 stars 0 forks source link

Running pickle.dumps or obj.__reduce__ multiple times will cause a segfault #183

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1. Execute pickle.dumps or obj._reduce__ multiple times (it can be the same 
object or different ones)

What is the expected output? What do you see instead?
Excepted: obj.__reduce__ should work.
Actual: segfault after a few times of running obj.__reduce__

What version of the product are you using? On what operating system?
Linux Netbook 3.5.0-25-generic #39-Ubuntu SMP Mon Feb 25 19:02:34 UTC 2013 i686 
i686 i686 GNU/Linux
Shedskin 0.9.3 / HEAD tree:3963356

Please provide any additional information below.

Running this code will reproduce the problem.

#blah
import blah as test
import pickle

blah = test.Blah(7)
for i in xrange(10):
    print i, blah.__reduce__()

Once again, thank you very much for this excellent project!
If you give me some pointers on where to start or how to debug this, I can help 
you.

Regards

Original issue reported on code.google.com by ernestof...@gmail.com on 3 Mar 2013 at 7:08

GoogleCodeExporter commented 9 years ago 
This is the output of that script:

0 (<built-in function __newobj__>, (<type 'blah.Blah'>,), (7,))
1 (<built-in function __newobj__>, (<type 'blah.Blah'>,), (7,))
2 (<built-in function __newobj__>, (<type 'blah.Blah'>,), (7,))
3 (<built-in function __newobj__>, (<type 'blah.Blah'>,), (7,))
4 (<built-in function __newobj__>, (<type 'blah.Blah'>,), (7,))
5 (<built-in function __newobj__>, (<type 'blah.Blah'>,), (7,))
6 (<built-in function __newobj__>, (<type 'blah.Blah'>,), (7,))
Segmentation fault

Original comment by ernestof...@gmail.com on 3 Mar 2013 at 7:09

GoogleCodeExporter commented 9 years ago 
thanks a lot! you could have a look here.. ^^

http://docs.python.org/2/c-api/intro.html#objects-types-and-reference-counts
http://docs.python.org/2/c-api/tuple.html
__ss_blah_Blah__reduce__ in blah.cpp
shedskin/extmod.py

Original comment by mark.duf...@gmail.com on 4 Mar 2013 at 8:21

GoogleCodeExporter commented 9 years ago 
This is the first time that I'm working the Python C Api so I might be wrong 
here, but this is what I think the problem is.

PyTuple_SetItem does not increase the ref counter and that's not a problem for 
all the newly created local objects.
The exception to that is this line:

print >>gv.out, '    PyTuple_SetItem(a, 0, (PyObject *)&%sObjectType);' % 
clname(cl)

So executing this line before solves that:

print >>gv.out, '    Py_INCREF((PyObject *)&%sObjectType);' % clname(cl)

I made this change and run my example and it works. I then modified it to be 
more aggresive and now I'm getting this:

Fatal Python error: deallocating None
Aborted

I'll keep working on it to see if I can find the reason of this.

The new example is:

#blah
import blah as test
import decimal
import pickle

blah = test.Blah(10, 20, 30)
blah2 = decimal.Decimal(10)

for i in xrange(1000):
    print i
    print pickle.loads(pickle.dumps(blah)).a
    print pickle.loads(pickle.dumps(blah)).b
    print pickle.loads(pickle.dumps(blah)).c
    print

I'm also attaching the diff for extmod.py

Also, there seems to be another issue with subclasses but I have to verify it.

I'll keep you informed, let me know if you find something on your side. If my 
change was correct, then it looks like the new problem will be harder to solve.

Regards

Original comment by ernestof...@gmail.com on 5 Mar 2013 at 1:11

Attachments:

GoogleCodeExporter commented 9 years ago 
great, thanks for the patch! I agree with your solution. now should I just 
commit it, or would you be interested in creating a patch using git? (using git 
format-patch I think, I always forget myself).

Original comment by mark.duf...@gmail.com on 5 Mar 2013 at 4:39

GoogleCodeExporter commented 9 years ago 
I would say that for this time, just commit it and I'll create a path using git 
for the next one.
I can't try it now but in theory this should patch the file:

patch -p1 -i Tb02.patch

Regards

Original comment by ernestof...@gmail.com on 5 Mar 2013 at 4:45

GoogleCodeExporter commented 9 years ago 
Do you have any thoughts on why the new problem might be happening?

Original comment by ernestof...@gmail.com on 5 Mar 2013 at 4:46

GoogleCodeExporter commented 9 years ago 
yes, look in __ss_blah_Blah__setstate__.. even None is refcounted in python!

Original comment by mark.duf...@gmail.com on 5 Mar 2013 at 4:53

GoogleCodeExporter commented 9 years ago 
btw, please also next time use separate patches for whitespace cleanups.

Original comment by mark.duf...@gmail.com on 5 Mar 2013 at 4:54

GoogleCodeExporter commented 9 years ago 
Sure. I don't use git in my projects (I use mercurial) so, what command with 
what parameters should I use to create a patch?

Original comment by ernestof...@gmail.com on 5 Mar 2013 at 5:13

GoogleCodeExporter commented 9 years ago 
git config --global user.name 'Dan O'Brien Muzyka'
git config --global user.email 'dan@danmuzyka.com'
git commit (make one or several commits)
git log (find the last commit id before these commits)
git format-patch (now there will be a few .patch files in the current dir)

the last two commands can probably me done in one, but I don't know.

Original comment by mark.duf...@gmail.com on 5 Mar 2013 at 5:25

GoogleCodeExporter commented 9 years ago 
I meant:

git format-patch commit-id (fill in the commit id from the previous step)

Original comment by mark.duf...@gmail.com on 5 Mar 2013 at 5:25

GoogleCodeExporter commented 9 years ago 
Both the previous change and a new one for the missing Py_INCREF for None are 
in the attached patch.

With regards to the whitespace changes, those were made automatically by my 
configuration in VIM. When I read that I had no idea what you meant, until I 
checked the patch.

Anyway, let me know if the patch is OK.

Original comment by ernestof...@gmail.com on 5 Mar 2013 at 11:24

Attachments:

GoogleCodeExporter commented 9 years ago 
pushed to master, thanks a lot! I also added your name to the main page.

Original comment by mark.duf...@gmail.com on 6 Mar 2013 at 10:01