diff --git a/spyder_kernels/comms/commbase.py b/spyder_kernels/comms/commbase.py index 20e3a8a3..800078f9 100644 --- a/spyder_kernels/comms/commbase.py +++ b/spyder_kernels/comms/commbase.py @@ -8,7 +8,8 @@ Class that handles communications between Spyder kernel and frontend. Comms transmit data in a list of buffers, and in a json-able dictionnary. -Here, we only support a buffer list with a single element. +Here, we only support json to avoid issues of compatibility between Python +versions. In the abstraction below, buffers is used to send bytes. The messages exchanged have the following msg_dict: @@ -19,8 +20,6 @@ } ``` -The buffer is generated by cloudpickle using `PICKLE_PROTOCOL = 2`. - To simplify the usage of messaging, we use a higher level function calling mechanism: - The `remote_call` method returns a RemoteCallHandler object @@ -35,33 +34,32 @@ 'call_name': The name of the function to be called, 'call_id': uuid to match the request to a potential reply, 'settings': A dictionnary of settings, - } - - The buffer encodes a dictionnary { 'call_args': The function args, 'call_kwargs': The function kwargs, - } + 'buffered_args': The args index that are in the buffers, + 'buffered_kwargs': the kwargs keys that are in the buffers + } + - The buffer contains any bytes in the arguments - If the 'settings' has `'blocking' = True`, a reply is sent. (spyder_msg_type = 'remote_call_reply'): - - The buffer contains the return value of the function. - The 'content' is a dict with: { - 'is_error': a boolean indicating if the return value is an - exception to be raised. - 'call_id': The uuid from above, - 'call_name': The function name (mostly for debugging) - } + 'is_error': a boolean indicating if the return value is an + exception to be raised. + 'call_id': The uuid from above, + 'call_name': The function name (mostly for debugging), + 'call_return_value': The return value of the function + } + - The buffer contains the return value if it is bytes """ -import cloudpickle -import pickle import logging import sys import uuid import traceback +import builtins logger = logging.getLogger(__name__) -DEFAULT_PICKLE_PROTOCOL = 4 - # Max timeout (in secs) for blocking calls TIMEOUT = 3 @@ -70,6 +68,32 @@ class CommError(RuntimeError): pass +def stacksummary_to_json(stack): + """StackSummary to json.""" + return [ + { + "filename": frame.filename, + "lineno": frame.lineno, + "name": frame.name, + "line": frame.line + } + for frame in stack + ] + + +def staksummary_from_json(stack): + """StackSummary from json.""" + traceback.StackSummary.from_list([ + ( + frame["filename"], + frame["lineno"], + frame["name"], + frame["line"] + ) + for frame in stack + ]) + + class CommsErrorWrapper(): def __init__(self, call_name, call_id): self.call_name = call_name @@ -77,6 +101,32 @@ def __init__(self, call_name, call_id): self.etype, self.error, tb = sys.exc_info() self.tb = traceback.extract_tb(tb) + def to_json(self): + """Create JSON representation.""" + return { + "call_name": self.call_name, + "call_id": self.call_id, + "etype": self.etype.__name__, + "args": self.error.args, + "tb": stacksummary_to_json(self.tb) + } + + @classmethod + def from_json(cls, json_data): + """Get a CommsErrorWrapper from a JSON representation.""" + instance = cls.__new__(cls) + instance.call_name = json_data["call_name"] + instance.call_id = json_data["call_id"] + etype = json_data["etype"] + instance.etype = getattr( + builtins, + etype, + type(etype, (Exception,), {}) + ) + instance.error = instance.etype(*json_data["args"]) + instance.tb = staksummary_from_json(json_data["tb"]) + return instance + def raise_error(self): """ Raise the error while adding informations on the callback. @@ -204,8 +254,9 @@ def remote_call(self, comm_id=None, callback=None, **settings): return RemoteCallFactory(self, comm_id, callback, **settings) # ---- Private ----- - def _send_message(self, spyder_msg_type, content=None, data=None, - comm_id=None): + def _send_message( + self, spyder_msg_type, content=None, comm_id=None, buffers=None + ): """ Publish custom messages to the other side. @@ -215,11 +266,10 @@ def _send_message(self, spyder_msg_type, content=None, data=None, The spyder message type content: dict The (JSONable) content of the message - data: any - Any object that is serializable by cloudpickle (should be most - things). Will arrive as cloudpickled bytes in `.buffers[0]`. comm_id: int the comm to send to. If None sends to all comms. + buffers: list(bytes) + a list of bytes to send. """ if not self.is_open(comm_id): raise CommError("The comm is not connected.") @@ -228,17 +278,9 @@ def _send_message(self, spyder_msg_type, content=None, data=None, msg_dict = { 'spyder_msg_type': spyder_msg_type, 'content': content, - 'pickle_protocol': self._comms[comm_id]['pickle_protocol'], - 'python_version': sys.version, - } - buffers = [cloudpickle.dumps( - data, protocol=self._comms[comm_id]['pickle_protocol'])] - self._comms[comm_id]['comm'].send(msg_dict, buffers=buffers) + } - def _set_pickle_protocol(self, protocol): - """Set the pickle protocol used to send data.""" - protocol = min(protocol, pickle.HIGHEST_PROTOCOL) - self._comms[self.calling_comm_id]['pickle_protocol'] = protocol + self._comms[comm_id]['comm'].send(msg_dict, buffers=buffers) @property def _comm_name(self): @@ -256,9 +298,8 @@ def _register_message_handler(self, message_id, handler): message_id : str The identifier for the message handler : callback - A function to handle the message. This is called with 3 arguments: + A function to handle the message. This is called with: - msg_dict: A dictionary with message information. - - buffer: The data transmitted in the buffer Pass None to unregister the message_id """ if handler is None: @@ -275,9 +316,8 @@ def _register_comm(self, comm): comm.on_close(self._comm_close) self._comms[comm.comm_id] = { 'comm': comm, - 'pickle_protocol': DEFAULT_PICKLE_PROTOCOL, 'status': 'opening', - } + } def _comm_close(self, msg): """Close comm.""" @@ -292,40 +332,35 @@ def _comm_message(self, msg): # Get message dict msg_dict = msg['content']['data'] - - # Load the buffer. Only one is supported. - try: - buffer = cloudpickle.loads(msg['buffers'][0]) - except Exception as e: - logger.debug( - "Exception in cloudpickle.loads : %s" % str(e)) - buffer = CommsErrorWrapper( - msg_dict['content']['call_name'], - msg_dict['content']['call_id']) - - msg_dict['content']['is_error'] = True - spyder_msg_type = msg_dict['spyder_msg_type'] + buffers = msg['buffers'] if spyder_msg_type in self._message_handlers: - self._message_handlers[spyder_msg_type]( - msg_dict, buffer) + self._message_handlers[spyder_msg_type](msg_dict, buffers) else: logger.debug("No such spyder message type: %s" % spyder_msg_type) - def _handle_remote_call(self, msg, buffer): + def _handle_remote_call(self, msg, buffers): """Handle a remote call.""" msg_dict = msg['content'] self.on_incoming_call(msg_dict) - if msg['content'].get('is_error', False): - # could not open the pickle - self._set_call_return_value(msg, buffer, is_error=True) - return try: + # read buffers + args = msg_dict['call_args'] + kwargs = msg_dict['call_kwargs'] + + if buffers: + for idx in msg_dict['buffered_args']: + args[idx] = buffers.pop(0) + for name in msg_dict['buffered_kwargs']: + kwargs[name] = buffers.pop(0) + assert len(buffers) == 0 + return_value = self._remote_callback( - msg_dict['call_name'], - buffer['call_args'], - buffer['call_kwargs']) + msg_dict['call_name'], + args, + kwargs + ) self._set_call_return_value(msg_dict, return_value) except Exception: exc_infos = CommsErrorWrapper( @@ -340,7 +375,7 @@ def _remote_callback(self, call_name, call_args, call_kwargs): raise CommError("No such spyder call type: %s" % call_name) - def _set_call_return_value(self, call_dict, data, is_error=False): + def _set_call_return_value(self, call_dict, return_value, is_error=False): """ A remote call has just been processed. @@ -350,21 +385,34 @@ def _set_call_return_value(self, call_dict, data, is_error=False): display_error = ('display_error' in settings and settings['display_error']) - if is_error and display_error: - data.print_error() + if is_error: + if display_error: + return_value.print_error() + return_value = return_value.to_json() send_reply = 'send_reply' in settings and settings['send_reply'] if not send_reply: # Nothing to send back return + + buffers = None + if isinstance(return_value, bytes): + buffers = [return_value] + return_value = None + content = { 'is_error': is_error, 'call_id': call_dict['call_id'], - 'call_name': call_dict['call_name'] + 'call_name': call_dict['call_name'], + 'call_return_value': return_value } - self._send_message('remote_call_reply', content=content, data=data, - comm_id=self.calling_comm_id) + self._send_message( + 'remote_call_reply', + content=content, + comm_id=self.calling_comm_id, + buffers=buffers + ) def _register_call(self, call_dict, callback=None): """ @@ -378,20 +426,18 @@ def _register_call(self, call_dict, callback=None): def on_outgoing_call(self, call_dict): """A message is about to be sent""" - call_dict["pickle_highest_protocol"] = pickle.HIGHEST_PROTOCOL return call_dict def on_incoming_call(self, call_dict): """A call was received""" - if "pickle_highest_protocol" in call_dict: - self._set_pickle_protocol(call_dict["pickle_highest_protocol"]) + pass - def _send_call(self, call_dict, call_data, comm_id): + def _send_call(self, call_dict, comm_id, buffers=None): """Send call.""" call_dict = self.on_outgoing_call(call_dict) self._send_message( - 'remote_call', content=call_dict, data=call_data, - comm_id=comm_id) + 'remote_call', content=call_dict, comm_id=comm_id, buffers=buffers + ) def _get_call_return_value(self, call_dict, comm_id): """ @@ -403,7 +449,6 @@ def _get_call_return_value(self, call_dict, comm_id): settings = call_dict['settings'] blocking = 'blocking' in settings and settings['blocking'] - if not blocking: return @@ -418,12 +463,12 @@ def _get_call_return_value(self, call_dict, comm_id): self._wait_reply(comm_id, call_id, call_name, timeout) - reply = self._reply_inbox.pop(call_id) + content = self._reply_inbox.pop(call_id) + return_value = content['call_return_value'] - if reply['is_error']: - return self._sync_error(reply['value']) - - return reply['value'] + if content['is_error']: + return self._sync_error(return_value) + return return_value def _wait_reply(self, comm_id, call_id, call_name, timeout): """ @@ -431,7 +476,7 @@ def _wait_reply(self, comm_id, call_id, call_name, timeout): """ raise NotImplementedError - def _handle_remote_call_reply(self, msg_dict, buffer): + def _handle_remote_call_reply(self, msg_dict, buffers): """ A blocking call received a reply. """ @@ -439,11 +484,20 @@ def _handle_remote_call_reply(self, msg_dict, buffer): call_id = content['call_id'] call_name = content['call_name'] is_error = content['is_error'] + return_value = content['call_return_value'] + + # Prepare return value + if is_error: + return_value = CommsErrorWrapper.from_json(return_value) + elif buffers: + assert len(buffers) == 1 + return_value = buffers[0] + content['call_return_value'] = return_value # Unexpected reply if call_id not in self._reply_waitlist: if is_error: - return self._async_error(buffer) + return self._async_error(return_value) else: logger.debug('Got an unexpected reply {}, id:{}'.format( call_name, call_id)) @@ -453,19 +507,15 @@ def _handle_remote_call_reply(self, msg_dict, buffer): # Async error if is_error and not blocking: - return self._async_error(buffer) + return self._async_error(return_value) # Callback if callback is not None and not is_error: - callback(buffer) + callback(return_value) # Blocking inbox if blocking: - self._reply_inbox[call_id] = { - 'is_error': is_error, - 'value': buffer, - 'content': content - } + self._reply_inbox[call_id] = content def _async_error(self, error_wrapper): """ @@ -515,21 +565,40 @@ def __call__(self, *args, **kwargs): """ Transmit the call to the other side of the tunnel. - The args and kwargs have to be picklable. + The args and kwargs have to be JSON-serializable or bytes. """ blocking = 'blocking' in self._settings and self._settings['blocking'] self._settings['send_reply'] = blocking or self._callback is not None + # The call will be serialized with json. The bytes are sent separately. + buffers = [] + buffered_args = [] + buffered_kwargs = [] + args = list(args) + + for i, arg in enumerate(args): + if isinstance(arg, bytes): + buffers.append(arg) + buffered_args.append(i) + args[i] = None + + for name in kwargs: + arg = kwargs[name] + if isinstance(arg, bytes): + buffers.append(arg) + buffered_kwargs.append(name) + kwargs[name] = None + call_id = uuid.uuid4().hex call_dict = { 'call_name': self._name, 'call_id': call_id, 'settings': self._settings, - } - call_data = { 'call_args': args, 'call_kwargs': kwargs, - } + 'buffered_args': buffered_args, + 'buffered_kwargs': buffered_kwargs + } if not self._comms_wrapper.is_open(self._comm_id): # Only an error if the call is blocking. @@ -538,6 +607,6 @@ def __call__(self, *args, **kwargs): logger.debug("Call to unconnected comm: %s" % self._name) return self._comms_wrapper._register_call(call_dict, self._callback) - self._comms_wrapper._send_call(call_dict, call_data, self._comm_id) + self._comms_wrapper._send_call(call_dict, self._comm_id, buffers) return self._comms_wrapper._get_call_return_value( call_dict, self._comm_id) diff --git a/spyder_kernels/comms/frontendcomm.py b/spyder_kernels/comms/frontendcomm.py index 9eaf2ee1..514d5404 100644 --- a/spyder_kernels/comms/frontendcomm.py +++ b/spyder_kernels/comms/frontendcomm.py @@ -176,8 +176,6 @@ def _comm_open(self, comm, msg): """ self.calling_comm_id = comm.comm_id self._register_comm(comm) - self._set_pickle_protocol( - msg['content']['data']['pickle_highest_protocol']) # IOPub might not be connected yet, keep sending messages until a # reply is received. @@ -196,7 +194,7 @@ def _async_error(self, error_wrapper): """ Send an async error back to the frontend to be displayed. """ - self.remote_call()._async_error(error_wrapper) + self.remote_call()._async_error(error_wrapper.to_json()) def _register_comm(self, comm): """ diff --git a/spyder_kernels/console/kernel.py b/spyder_kernels/console/kernel.py index 15a407aa..da595371 100644 --- a/spyder_kernels/console/kernel.py +++ b/spyder_kernels/console/kernel.py @@ -20,6 +20,7 @@ import traceback import tempfile import threading +import cloudpickle # Third-party imports from ipykernel.ipkernel import IPythonKernel @@ -30,6 +31,7 @@ # Local imports import spyder_kernels +from spyder_kernels.comms.commbase import stacksummary_to_json from spyder_kernels.comms.frontendcomm import FrontendComm from spyder_kernels.comms.decorators import ( register_comm_handlers, comm_handler) @@ -257,7 +259,7 @@ def get_current_frames(self, ignore_internal_threads=True): """Get the current frames.""" ignore_list = self.get_system_threads_id() main_id = threading.main_thread().ident - frames = {} + stack_dict = {} thread_names = {thread.ident: thread.name for thread in threading.enumerate()} @@ -274,8 +276,12 @@ def get_current_frames(self, ignore_internal_threads=True): thread_name = thread_names[thread_id] else: thread_name = str(thread_id) - frames[thread_name] = stack - return frames + + # Transform stack in a dict because FrameSummary objects + # are not compatible between versions of Python + stack_dict[thread_name] = stacksummary_to_json(stack) + + return stack_dict # --- For the Variable Explorer @comm_handler @@ -346,14 +352,22 @@ def get_var_properties(self): return None @comm_handler - def get_value(self, name): + def get_value(self, name, encoded=False): """Get the value of a variable""" ns = self.shell._get_current_namespace() - return ns[name] + value = ns[name] + if encoded: + # Encode with cloudpickle + value = cloudpickle.dumps(value) + return value @comm_handler - def set_value(self, name, value): + def set_value(self, name, value, encoded=False): """Set the value of a variable""" + if encoded: + # Decode_value + value = cloudpickle.loads(value) + ns = self.shell._get_reference_namespace(name) ns[name] = value self.log.debug(ns) @@ -705,7 +719,7 @@ def set_special_kernel(self, special): exec("from pylab import *", self.shell.user_ns) self.shell.special = special return - + if special == "sympy": import sympy # noqa sympy_init = "\n".join([ diff --git a/spyder_kernels/console/shell.py b/spyder_kernels/console/shell.py index 48cecaf3..5a9c1a29 100644 --- a/spyder_kernels/console/shell.py +++ b/spyder_kernels/console/shell.py @@ -26,6 +26,7 @@ from spyder_kernels.customize.namespace_manager import NamespaceManager from spyder_kernels.customize.spyderpdb import SpyderPdb from spyder_kernels.customize.code_runner import SpyderCodeRunner +from spyder_kernels.comms.commbase import stacksummary_to_json from spyder_kernels.comms.decorators import comm_handler from spyder_kernels.utils.mpl import automatic_backend @@ -271,7 +272,9 @@ def showtraceback(self, exc_tuple=None, filename=None, tb_offset=None, if not exception_only: try: etype, value, tb = self._get_exc_info(exc_tuple) - stack = traceback.extract_tb(tb.tb_next) + etype = etype.__name__ + value = value.args + stack = stacksummary_to_json(traceback.extract_tb(tb.tb_next)) self.kernel.frontend_call(blocking=False).show_traceback( etype, value, stack) except Exception: diff --git a/spyder_kernels/console/tests/test_console_kernel.py b/spyder_kernels/console/tests/test_console_kernel.py index b7bbbc6b..60f240d3 100644 --- a/spyder_kernels/console/tests/test_console_kernel.py +++ b/spyder_kernels/console/tests/test_console_kernel.py @@ -1272,13 +1272,12 @@ def test_interrupt(): """ # Command to start the kernel cmd = "from spyder_kernels.console import start; start.main()" - import pickle with setup_kernel(cmd) as client: kernel_comm = CommBase() # Create new comm and send the highest protocol comm = Comm(kernel_comm._comm_name, client) - comm.open(data={'pickle_highest_protocol': pickle.HIGHEST_PROTOCOL}) + comm.open(data={}) comm._send_channel = client.control_channel kernel_comm._register_comm(comm) @@ -1328,13 +1327,12 @@ def test_enter_debug_after_interruption(): """ # Command to start the kernel cmd = "from spyder_kernels.console import start; start.main()" - import pickle with setup_kernel(cmd) as client: kernel_comm = CommBase() # Create new comm and send the highest protocol comm = Comm(kernel_comm._comm_name, client) - comm.open(data={'pickle_highest_protocol': pickle.HIGHEST_PROTOCOL}) + comm.open(data={}) comm._send_channel = client.control_channel kernel_comm._register_comm(comm) @@ -1393,7 +1391,7 @@ def test_django_settings(kernel): def test_hard_link_pdb(tmpdir): """ - Test that breakpoints on a file are recognised even when the path is + Test that breakpoints on a file are recognised even when the path is different. """ # Create a file and a hard link @@ -1403,10 +1401,10 @@ def test_hard_link_pdb(tmpdir): os.mkdir(folder) hard_link = folder.join("file.py") os.link(d, hard_link) - + # Make sure both paths point to the same file assert os.path.samefile(d, hard_link) - + # Make sure canonic returns the same path for a single file pdb_obj = SpyderPdb() assert pdb_obj.canonic(str(d)) == pdb_obj.canonic(str(hard_link)) diff --git a/spyder_kernels/customize/namespace_manager.py b/spyder_kernels/customize/namespace_manager.py index f758cf27..c3fcbcba 100755 --- a/spyder_kernels/customize/namespace_manager.py +++ b/spyder_kernels/customize/namespace_manager.py @@ -76,7 +76,6 @@ def __enter__(self): self.ns_globals = main_mod.__dict__ self.ns_locals = None - # Needed to allow pickle to reference main if '__main__' in sys.modules: self._previous_main = sys.modules['__main__'] sys.modules['__main__'] = main_mod diff --git a/spyder_kernels/customize/spyderpdb.py b/spyder_kernels/customize/spyderpdb.py index 8fd4e975..a522afa5 100755 --- a/spyder_kernels/customize/spyderpdb.py +++ b/spyder_kernels/customize/spyderpdb.py @@ -24,6 +24,7 @@ from IPython.core.inputtransformer2 import TransformerManager import spyder_kernels +from spyder_kernels.comms.commbase import stacksummary_to_json from spyder_kernels.comms.frontendcomm import CommError, frontend_request from spyder_kernels.customize.utils import ( path_is_library, capture_last_Expr, exec_encapsulate_locals @@ -829,9 +830,10 @@ def get_pdb_state(self): if self.pdb_publish_stack: # Publish Pdb stack so we can update the Debugger plugin on Spyder - pdb_stack = traceback.StackSummary.extract(self.stack) + pdb_stack = stacksummary_to_json( + traceback.StackSummary.extract(self.stack) + ) pdb_index = self.curindex - skip_hidden = getattr(self, 'skip_hidden', False) if skip_hidden: