msg223161 - (view) |
Author: (ppperry) |
Date: 2014-07-16 00:13 |
In IDLE:
>>> code = compile("dummy_code", "<test>", "exec")
>>> pickle.dumps(code)
"cidlelib.rpc\nunpickle_code\np0\n(S'c\\x00\\x00\\x00\\x00\\x00\\x00 \\x00\\x00\\x01\\x00\\x00\\x00@\\x00\\x00\\x00s\\x08\\x00\\x00\\x00e\\x00\\x00\\x01d\\x00\\x00S(\\x01\\x00\\x00\\x00N(\\x01\\x00\\x00\\x00t\\n\\x00\\x00\\x00dummy_code(\\x00\\x00\\x00\\x00(\\x00\\x00\\x00\\x00(\\x00\\x00\\x00\\x00s\\x06\\x00\\x00\\x00<test>t\\x08\\x00\\x00\\x00<module>\\x01\\x00\\x00\\x00s\\x00\\x00\\x00\\x00'\np1\ntp2\nRp3\n."
Outside of IDLE:
>>> code = compile("dummy_code", "<test>", "exec")
>>> pickle.dumps(code)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "C:\Python27\lib\pickle.py", line 1374, in dumps
Pickler(file, protocol).dump(obj)
File "C:\Python27\lib\pickle.py", line 224, in dump
self.save(obj)
File "C:\Python27\lib\pickle.py", line 306, in save
rv = reduce(self.proto)
File "C:\Python27\lib\copy_reg.py", line 70, in _reduce_ex
raise TypeError, "can't pickle %s objects" % base.__name__
TypeError: can't pickle code objects
Also, the error probably should be a pickle.PicklingError, not a TypeError.
|
msg223173 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-07-16 05:42 |
Code is really a code object, so the compile does not seem to be the problem. In 2.7, on Win7, I get exactly the same output after removing the possibly spurious space in the string posted. (ppperry), what system (OS) are you using. (In the future, please report both system and Python version).
3.4 gives a mild variation
b'\x80\x03cidlelib.rpc\nunpickle_code\nq\x00CZ\xe3\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00@\x00\x00\x00s\x08\x00\x00\x00e\x00\x00\x01d\x00\x00S)\x01N)\x01Z\ndummy_code\xa9\x00r\x01\x00\x00\x00r\x01\x00\x00\x00z\x06<test>\xda\x08<module>\x01\x00\x00\x00s\x00\x00\x00\x00q\x01\x85q\x02Rq\x03.'
I ran both test_pickle and test_pickletools from both the command line
>python -m test -v test_pickletools
and from an Idle editor with F5, with no errors and apparently identical output.
import pickle
code = compile("dummy_code", "this is a file name", "exec")
print(type(code))
print('pickle: ', pickle.dumps(code))
>>> produces
<class 'code'>
pickle: b'\x80\x03cidlelib.rpc ... \x13this is a file name\...'
which shows that the bytes come from picke, not from a garbled traceback.
Since the bytes look like an actual pickle, I tried unpickling and it works:
import pickle
code = compile("'a string'", "", "eval")
pick = pickle.dumps(code)
code2 = pickle.loads(pick)
print(eval(code), eval(code2))
>>>
a string a string
import pickle
code1 = compile("print('a string')", "", "exec")
pick = pickle.dumps(code1)
code2 = pickle.loads(pick)
exec(code1)
exec(code2)
>>>
a string
a string
I do not see any Idle bug here, so this might be closed unless you want to make this a pickle enhancement issue to work on code objects in the standard interpreter. The 3.4 interpreter traceback gives more info on why the pickle fails there.
>>> code0 = compile("'abc'", '', 'eval')
>>> pick = pickle.dumps(code0)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
_pickle.PicklingError: Can't pickle <class 'code'>: attribute lookup code on builtins failed
In Idle, __builtins__.code fails, just as in the interpreter, so pickle is doing something different, and more successful, in the slightly altered Idle user-process execution environment.
|
msg223176 - (view) |
Author: PCManticore (Claudiu.Popa) * |
Date: 2014-07-16 06:10 |
It works in IDLE because it registers a custom pickling for code objects, in idlelib.rpc:
copyreg.pickle(types.CodeType, pickle_code, unpickle_code)
where pickle_code / unpickle_code calls marshal.dumps/loads.
Although, I admit that this is weird. If idlelib.rpc is using this for transferring data between RPC instances, that's okay, but leaking the behaviour in the IDLE's interactive interpreter is not that okay, because leads to different results and expectancies between IDLE and Python's interactive interpreter.
|
msg223193 - (view) |
Author: Martin v. Löwis (loewis) * |
Date: 2014-07-16 12:09 |
I agree with Claudiu. IDLE should pickle with a private dispatch_table.
|
msg223201 - (view) |
Author: PCManticore (Claudiu.Popa) * |
Date: 2014-07-16 13:53 |
Maybe something like the attached patch. It doesn't have tests, though, I didn't find any tests for the idlelib.rpc anyway.
|
msg223205 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-07-16 14:12 |
Instead of copying dispatch_table, use ChainMap.
|
msg223208 - (view) |
Author: PCManticore (Claudiu.Popa) * |
Date: 2014-07-16 14:18 |
Thanks, Serhiy.
|
msg223243 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-07-16 17:52 |
There is no unittest module for rpc yet.
Should the pickle test include a test that pickling a code object fails, and with the proper exception? Is ppperry correct about PicklingError?
Chainmap is new in 3.3 so 2.7 would need the copy version of the patch.
|
msg223244 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-07-16 17:55 |
ppperry: Component Windows (or Macintosh) means Windows (or Mac) specific. The rpc code is general to all systems.
|
msg223255 - (view) |
Author: PCManticore (Claudiu.Popa) * |
Date: 2014-07-16 19:26 |
TypeError is raised only in Python 2, in Python 3 it's PicklingError.
|
msg228887 - (view) |
Author: PCManticore (Claudiu.Popa) * |
Date: 2014-10-09 18:13 |
Terry, can I do something to move this issue forward?
|
msg228906 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-10-09 20:23 |
Here are the issues for me.
1. Except for your interest, this is a lower priority for me that most all of the other 125 Idle issues.
2. I am not familiar with pickling and wonder about the following in the patch. It deletes a statememt
copyreg.pickle(types.CodeType, pickle_code, unpickle_code)
that registers two functions. But the new code only registers pickle_code. Will
message = pickle.loads(packet)
then work? Maybe, since pickle_code has this.
return unpickle_code, (ms,)
Is this normal? Is registring unpickle_code not needed? Would it make any sense to wrap code objects in a CodePickler class with __getstate__ and __setstate__ methods?
3. The interprocess communication implemented in rpc.py is central to Idle's execution of user code. So I would want to be confident that a patch does not break it. What I would like is a test script (executed by hand is ok) that works now, fails with the deletion of the copyreg statement, and works again with the rest of the patch.
4. I have no idea if or from where a code object is sent. One way to search would be to grep idlelib for rpc calls. Another would be to add a print statement to pickle_code, start Idle from the console with 'python -m idlelib', and run through all the menu options that involve the user process while watching the console for messages.
So you could try one of the searches methods to find a test. Or you can wait for someone who currently understands pickle well enough to review and apply the patches without a test.
|
msg228912 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-10-09 21:13 |
2. It is normal. The third argument of copyreg.pickle() is not used now.
The patch LGTM.
|
msg228917 - (view) |
Author: PCManticore (Claudiu.Popa) * |
Date: 2014-10-09 21:28 |
Thank you for your feedback, Terry.
1. IDLE is behaving differently than the builtin interpreter. It should be higher priority, because it
leads beginners into believing that code objects are picklable.
2. No, wrapping code objects in a CodePickler with __getstate__ et al will not work. What is important in my implementation of CodePickler is that the dispatch_table is private, so it will not leak private reduction function, as it happens right now.
3. That doesn't make sense. What works now is pickling of code objects, removing the copyreg call, without my patch, will break the rpc as well. To test this, comment the copyreg.pickle call in idlelib.rpc and just define a function in IDLE, you'll get a PicklingError. I added a small test file, which can be used to test what happens now and what happens after my patch.
4. Better, here's a traceback which can explain from where codes are sent. The traceback is from inside pickle_code, using code.InteractiveInterpreter.
File "C:\Python34\lib\idlelib\PyShell.py", line 1602, in main
root.mainloop()
File "C:\Python34\lib\tkinter\__init__.py", line 1069, in mainloop
self.tk.mainloop(n)
File "C:\Python34\lib\tkinter\__init__.py", line 1487, in __call__
return self.func(*args)
File "C:\Python34\lib\idlelib\MultiCall.py", line 179, in handler
r = l[i](event)
File "C:\Python34\lib\idlelib\PyShell.py", line 1188, in enter_callback
self.runit()
File "C:\Python34\lib\idlelib\PyShell.py", line 1229, in runit
more = self.interp.runsource(line)
File "C:\Python34\lib\idlelib\PyShell.py", line 671, in runsource
return InteractiveInterpreter.runsource(self, source, filename)
File "C:\Python34\lib\code.py", line 74, in runsource
self.runcode(code)
File "C:\Python34\lib\idlelib\PyShell.py", line 762, in runcode
(code,), {})
File "C:\Python34\lib\idlelib\rpc.py", line 256, in asyncqueue
self.putmessage((seq, request))
File "C:\Python34\lib\idlelib\rpc.py", line 348, in putmessage
s = pickle.dumps(message)
|
msg228932 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-10-09 23:54 |
Wanting to make sure that a patch does not break Idle makes perfect sense to me. As it turns out, running *any* code from the editor (even empty) works for these patches. Idle compiles the code to a code object *in the Idle process* and if no compile errors, ships the code object to the user process via rpc to be executed in the user process. I comfirmed this with a print inside pickle_code. With this clear, I have downloaded both patches to test on 2.7 and 3.4.
|
msg229046 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-10-10 23:35 |
New changeset 90c62e1f3658 by Terry Jan Reedy in branch '3.4':
Issue #21986: Idle now matches interpreter in not pickling user code objects.
https://hg.python.org/cpython/rev/90c62e1f3658
New changeset cb94764bf8be by Terry Jan Reedy in branch 'default':
Merge with 3.4: #21986, don't pickle user code objects.
https://hg.python.org/cpython/rev/cb94764bf8be
|
msg229048 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-10-10 23:39 |
Code objects get wrapped in 3 tuple layers before being 'pickled', so a private dispatch is the easiet solution. Since copyreg.dispatch_table has only two items to copy, I pushed a version of the first patch. Since private dispatch tables are new in 3.3 and not in 2.7, I withdraw the idea of patching 2.7.
|
msg229139 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-10-12 11:14 |
It would be good to add a test of rpc.dumps().
Current code copies copyreg.dispatch_table at the moment of rpc import. But it
can be changed later and these change will not affect private copy. For example
the re module adds pickleability of compiled regex patterns. I think the use
of ChainMap is more safe solution.
|
msg229214 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2014-10-13 02:46 |
I agree that a test for dumps would be a good idea. Ditto for hundreds of other untested functions. I don't see this one as a priority.
PyShell imports non-idlelib modules, including re, before idlelib modules, such as rpc. So the re addition is there, though I strongly doubt that compiled regexs are every sent to the user process. Since about 10 different messages of different types are sent as part of startup, the fact that Idle does start is strong evidence that dumps and much of the rest of rpc is ok. I would start a test of rpc by documenting the protocol and listing the messages types so at least one of each could be tested.
|
msg229225 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-10-13 07:21 |
> PyShell imports non-idlelib modules, including re, before idlelib modules,
> such as rpc. So the re addition is there, though I strongly doubt that
> compiled regexs are every sent to the user process.
But we can't guarantee that this alway will be so. Other stdlib modules can
register picklers, and IDLE can import them in future. This code is not
protected against future changes in other places of the code.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:06 | admin | set | github: 66185 |
2014-10-13 07:21:34 | serhiy.storchaka | set | messages:
+ msg229225 |
2014-10-13 02:46:33 | terry.reedy | set | messages:
+ msg229214 |
2014-10-12 11:14:48 | serhiy.storchaka | set | messages:
+ msg229139 |
2014-10-10 23:40:04 | terry.reedy | set | assignee: terry.reedy |
2014-10-10 23:39:54 | terry.reedy | set | status: open -> closed versions:
- Python 2.7 messages:
+ msg229048
resolution: fixed stage: patch review -> resolved |
2014-10-10 23:35:03 | python-dev | set | nosy:
+ python-dev messages:
+ msg229046
|
2014-10-09 23:54:21 | terry.reedy | set | messages:
+ msg228932 |
2014-10-09 21:28:26 | Claudiu.Popa | set | files:
+ test.py
messages:
+ msg228917 |
2014-10-09 21:25:41 | serhiy.storchaka | set | title: Pickleability of code objects is inconsistent -> Idle: disable pickleability of user code objects |
2014-10-09 21:13:36 | serhiy.storchaka | set | messages:
+ msg228912 title: Idle: disable pickleability of user code objects -> Pickleability of code objects is inconsistent |
2014-10-09 20:32:35 | terry.reedy | set | title: Pickleability of code objects is inconsistent -> Idle: disable pickleability of user code objects |
2014-10-09 20:23:36 | terry.reedy | set | messages:
+ msg228906 |
2014-10-09 18:13:06 | Claudiu.Popa | set | messages:
+ msg228887 |
2014-08-25 08:44:39 | Claudiu.Popa | set | stage: needs patch -> patch review |
2014-07-16 19:26:51 | Claudiu.Popa | set | messages:
+ msg223255 |
2014-07-16 17:55:40 | terry.reedy | set | messages:
+ msg223244 |
2014-07-16 17:52:35 | terry.reedy | set | messages:
+ msg223243 components:
+ IDLE, - Library (Lib), Windows |
2014-07-16 14:18:29 | Claudiu.Popa | set | files:
+ issue21986.patch
messages:
+ msg223208 |
2014-07-16 14:12:19 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg223205
|
2014-07-16 13:53:40 | Claudiu.Popa | set | files:
+ idlelib.patch keywords:
+ patch messages:
+ msg223201
|
2014-07-16 13:34:09 | ppperry | set | components:
+ Windows |
2014-07-16 12:09:48 | loewis | set | nosy:
+ loewis messages:
+ msg223193
|
2014-07-16 06:10:40 | Claudiu.Popa | set | nosy:
+ Claudiu.Popa messages:
+ msg223176
|
2014-07-16 05:42:24 | terry.reedy | set | versions:
+ Python 2.7, Python 3.4, Python 3.5 nosy:
+ pitrou
messages:
+ msg223173
components:
+ Library (Lib), - IDLE stage: needs patch |
2014-07-16 03:06:29 | pitrou | set | nosy:
+ terry.reedy, kbk, roger.serwy
|
2014-07-16 00:47:19 | ppperry | set | type: behavior |
2014-07-16 00:13:43 | ppperry | create | |