Issue3657
Created on 2008-08-24 07:01 by nnorwitz, last changed 2009-01-24 21:13 by mark.dickinson.
| Messages (14) | |||
|---|---|---|---|
| msg71830 - (view) | Author: Neal Norwitz (nnorwitz) | Date: 2008-08-24 07:01 | |
test_pickletools fails sporadically on at least two platforms I've seen. http://www.python.org/dev/buildbot/all/x86%20gentoo%20trunk/builds/4120/step-test/0 http://www.python.org/dev/buildbot/all/ppc%20Debian%20unstable%20trunk/builds/1908/step-test/0 File "/home/buildslave/python-trunk/trunk.norwitz-x86/build/Lib/pickletools.py", line ?, in pickletools.__test__.disassembler_test Failed example: dis(pickle.dumps(random.random, 0)) Expected: 0: c GLOBAL 'random random' 15: p PUT 0 18: . STOP highest protocol among opcodes = 0 Got: 0: c GLOBAL 'bsddb.test.test_thread random' 31: p PUT 0 34: . STOP highest protocol among opcodes = 0 ********************************************************************** 1 items had failures: 1 of 25 in pickletools.__test__.disassembler_test |
|||
| msg71898 - (view) | Author: Neal Norwitz (nnorwitz) | Date: 2008-08-24 23:04 | |
The valgrind errors below are possibly related. Conditional jump or move depends on uninitialised value(s) PyUnicodeUCS2_EncodeUTF8 (unicodeobject.c:2216) _PyUnicode_AsString (unicodeobject.c:1417) save (_pickle.c:930) Pickler_dump (_pickle.c:2292) Conditional jump or move depends on uninitialised value(s) PyUnicodeUCS2_EncodeUTF8 (unicodeobject.c:2220) _PyUnicode_AsString (unicodeobject.c:1417) save (_pickle.c:930) Pickler_dump (_pickle.c:2292) Conditional jump or move depends on uninitialised value(s) PyUnicodeUCS2_EncodeUTF8 (unicodeobject.c:2227) _PyUnicode_AsString (unicodeobject.c:1417) save (_pickle.c:930) Pickler_dump (_pickle.c:2292) Conditional jump or move depends on uninitialised value(s) PyUnicodeUCS2_EncodeUTF8 (unicodeobject.c:2229) _PyUnicode_AsString (unicodeobject.c:1417) save (_pickle.c:930) Pickler_dump (_pickle.c:2292) Conditional jump or move depends on uninitialised value(s) PyUnicodeUCS2_EncodeUTF8 (unicodeobject.c:2233) _PyUnicode_AsString (unicodeobject.c:1417) save (_pickle.c:930) Pickler_dump (_pickle.c:2292) |
|||
| msg71904 - (view) | Author: Neal Norwitz (nnorwitz) | Date: 2008-08-24 23:50 | |
Indeed. The problem was an incorrect conversion of str -> unicode, instead of converting to bytes. On getting the buffer from unicode, it tried to read data which was uninitialized. Hmmm, this fix is for 3.0 only, but the problem is happening in 2.6. Leaving open. Committed revision 66021. |
|||
| msg71910 - (view) | Author: Neal Norwitz (nnorwitz) | Date: 2008-08-25 06:39 | |
It seems that if the tests are run in this order: ./python -E -tt ./Lib/test/regrtest.py -u all test_xmlrpc test_ctypes test_json test_bsddb3 test_pickletools The error will trigger consistently. That is in 2.6 with a debug build on a dual cpu box. A debug build of 3.0 on the same machine did not fail though I don't know if 3.0 has this problem. I was unable to prune the list further. The 3 tests (xmlrpc, ctypes and json) can be run in any order prior to bsdb3 and then pickletools. |
|||
| msg72673 - (view) | Author: Barry A. Warsaw (barry) | Date: 2008-09-06 17:24 | |
Neal, can you verify that this is still a problem now that bsddb has been removed? The tests, run in the order of the last comment, succeed for me in both 2.6 and 3.0, debug build or not, on both my single processor Ubuntu box and dual core Mac. |
|||
| msg73061 - (view) | Author: Benjamin Peterson (benjamin.peterson) | Date: 2008-09-11 22:02 | |
I can reproduce this on 2.6. |
|||
| msg73081 - (view) | Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) | Date: 2008-09-12 11:34 | |
The explanation is actually simple, do not blame bsddb :-)
random.random is a built-in method, so its __module__ attribute is not
set. To find it, pickle.whichmodule loops through all sys.modules, and
pick the first one that contains the function.
I reproduced the problem with a simple file:
# someModule.py
from random import random
then, start python and:
>>> import someModule, random, pickle
>>> pickle.dumps(random.random, 0)
'csomeModule\nrandom\np0\n.'
You may have to change the name of "someModule", to be sure that it
appears before "random" in sys.modules. Tested on Windows and x86_64 Linux.
To correct this, one direction would be to search only built-in or
extension modules; for bound methods (random.random.__self__ is not
None), try to dump separately the instance and the method name (but
getattr(random._inst, 'random') is not random.random).
Or simply change the test...
|
|||
| msg73082 - (view) | Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) | Date: 2008-09-12 12:05 | |
I'm not sure that pickling random.random is a good idea; did you try to pickle the random.seed function? Their definition look very similar (at the end of random.py: _inst = Random() seed = _inst.seed random = _inst.random ) but Random.seed is a python method, whereas Random.random is inherited from _randommodule.c. |
|||
| msg73126 - (view) | Author: Tim Peters (tim_one) | Date: 2008-09-12 19:28 | |
No thought went into picking random.random in the test -- it was just a
random ;-) choice. Amaury's analysis of the source of non-determinism
is on target, and the easiest fix is to pick a coded-in-Python function
to pickle instead. I suggest, e.g., changing the sporadically failing
doctest to:
>>> import pickletools
>>> dis(pickle.dumps(pickletools.dis, 0))
0: c GLOBAL 'pickletools dis'
17: p PUT 0
20: . STOP
highest protocol among opcodes = 0
|
|||
| msg73138 - (view) | Author: Antoine Pitrou (pitrou) | Date: 2008-09-12 20:21 | |
But it still means pickling a function/method defined in a builtin extension module can give wrong results, doesn't it deserve being fixed? |
|||
| msg73142 - (view) | Author: Tim Peters (tim_one) | Date: 2008-09-12 21:40 | |
Amaury, yes, it would be nice if pickle were more reliable here. But that should be the topic of a different tracker issue. Despite the Title, this issue is really about sporadic pickletools.py failures, and the pickletools tests are trying to test pickletools, not pickle.py (or cPickle). Changing the test as suggested makes it reliably test what it's trying to test (namely that pickletools.dis() produces sensible output for pickle's GLOBAL opcode). Whether pickle/cPickle should do a better job of building GLOBAL opcodes in the first place is a distinct issue. In any case, since pickle/cPickle have worked this way forever, and the only known bad consequence to date is accidental sporadic test failures in pickletools.py, the underlying pickle/cPickle issue shouldn't be a release blocker regardless. |
|||
| msg73165 - (view) | Author: Tim Peters (tim_one) | Date: 2008-09-13 04:14 | |
BTW, note that the Title of this issue is misleading:
pickle.whichmodule() uses object identity ("is":
if ... getattr(module, funcname, None) is func:
) to determine whether the given function object is supplied by a
module, so it's /not/ the case that a "wrong" function can be pickled.
The worst that can happen is that the correct function is pickled but
obtained from a possibly surprising module. For example, random.random
can't be confused with any other function named "random".
I expect this is why nobody has ever complained about it: unless you're
looking at the strings embedded in the pickle GLOBAL opcode, it's
unlikely to have a visible consequence.
It would still be nice if pickle could identify "the most natural"
module for a given function, but hard to make a case that doing so would
be much more than /just/ "nice".
|
|||
| msg73174 - (view) | Author: Antoine Pitrou (pitrou) | Date: 2008-09-13 10:59 | |
> I expect this is why nobody has ever complained about it: unless you're > looking at the strings embedded in the pickle GLOBAL opcode, it's > unlikely to have a visible consequence. Well, it may have a consequence if pickle picks the "random" function from a third-party module named "foobar", and you give the pickle to a friend and expect it to work for him while he hasn't installed module "foobar". |
|||
| msg80479 - (view) | Author: Mark Dickinson (mark.dickinson) | Date: 2009-01-24 21:13 | |
I just committed Tim's suggested change in r68906. This seemed a no- brainer, regardless of what should be done about pickle.whichmodule. One fewer sporadic buildbot failure sounds like a good thing to me. (I hadn't noticed the pickletools failures until just after I committed a pickle module change, so I was rather thrown until Antoine directed me here...) |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2009-01-24 21:13:20 | mark.dickinson | set | messages: + msg80479 |
| 2009-01-24 20:00:19 | mark.dickinson | set | nosy: + mark.dickinson |
| 2008-09-13 10:59:09 | pitrou | set | messages: + msg73174 |
| 2008-09-13 04:14:46 | tim_one | set | messages: + msg73165 |
| 2008-09-12 23:03:03 | barry | set | priority: release blocker -> high |
| 2008-09-12 21:40:25 | tim_one | set | messages: + msg73142 |
| 2008-09-12 20:21:02 | pitrou | set | nosy:
+ pitrou messages: + msg73138 |
| 2008-09-12 20:19:16 | alexandre.vassalotti | set | nosy: + alexandre.vassalotti |
| 2008-09-12 19:28:54 | tim_one | set | nosy:
+ tim_one messages: + msg73126 |
| 2008-09-12 12:05:24 | amaury.forgeotdarc | set | messages: + msg73082 |
| 2008-09-12 11:34:58 | amaury.forgeotdarc | set | nosy:
+ amaury.forgeotdarc messages: + msg73081 |
| 2008-09-11 22:02:42 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages: + msg73061 |
| 2008-09-09 13:14:36 | barry | set | priority: deferred blocker -> release blocker |
| 2008-09-06 17:24:23 | barry | set | priority: release blocker -> deferred blocker resolution: works for me messages: + msg72673 nosy: + barry |
| 2008-08-25 06:40:00 | nnorwitz | set | messages: + msg71910 |
| 2008-08-24 23:50:25 | nnorwitz | set | assignee: nnorwitz messages: + msg71904 |
| 2008-08-24 23:04:16 | nnorwitz | set | messages: + msg71898 |
| 2008-08-24 07:01:47 | nnorwitz | create | |