Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RecursionError when unpickling message object #804

Closed
fpetry opened this issue Apr 2, 2020 · 2 comments
Closed

RecursionError when unpickling message object #804

fpetry opened this issue Apr 2, 2020 · 2 comments
Labels
Milestone

Comments

@fpetry
Copy link

fpetry commented Apr 2, 2020

Python 3.7.5
python-can 3.3.2

To hand over messages to another process I intended to use multiprocessing.JoinableQueue.
I stumbled over a RecursionError and tracked it down to the pickleing that is used to serialize the objects (see example code).

The error occurs when calling loads() on the serialized message.

I'm not a python expert and found some clues regarding the __getattr__ method. Maybe a custom __getattr__ method is needed for the message objects?

Is there another way to hand over the emssage objects to another process?

#!/usr/bin/env python3
import can
import logging
import pickle

class CanReader():
    """
    Reads messages from a configured can bus and puts them into a queue for further processing
    """

    def __init__(self):
        LOG_FORMAT = '%(asctime)s:%(threadName)s:%(levelname)s: %(message)s'
        logging.basicConfig(format=LOG_FORMAT, level=logging.INFO)


    def run(self):
        can.rc['interface'] = 'socketcan_ctypes'
        can.rc['channel'] = "vcan0"

        can_bus = can.interface.Bus()

        try:
            for can_msg in can_bus:
                if can_msg is  None:
                    continue

                pick = pickle.dumps(can_msg)
                logging.info("pickled")
                unpick = pickle.loads(pick)
                logging.info("unpickled")

        except KeyboardInterrupt:
            logging.error("KeyboardInterrupt - Stopping CanReader")

        return

if __name__ == "__main__":
    canReader = CanReader()
    canReader.run()

Console output:

/usr/bin/python3.7 /XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX/SerializationTest.py
2020-04-02 13:35:59,352:MainThread:INFO: Created a socket
2020-04-02 13:35:59,389:MainThread:INFO: pickled
Traceback (most recent call last):
  File "/XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX/SerializationTest.py", line 39, in <module>
    canReader.run()
  File "/XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX/SerializationTest.py", line 29, in run
    unpick = pickle.loads(pick)
  File "/home/XXXXXX/.local/lib/python3.7/site-packages/can/message.py", line 59, in __getattr__
    return self._dict[key]
  File "/home/XXXXXX/.local/lib/python3.7/site-packages/can/message.py", line 59, in __getattr__
    return self._dict[key]
  File "/home/XXXXXX/.local/lib/python3.7/site-packages/can/message.py", line 59, in __getattr__
    return self._dict[key]
  [Previous line repeated 494 more times]
  File "/home/XXXXXX/.local/lib/python3.7/site-packages/can/message.py", line 58, in __getattr__
    warnings.warn("Custom attributes of messages are deprecated and will be removed in 4.0", DeprecationWarning)
RecursionError: maximum recursion depth exceeded while calling a Python object

Process finished with exit code 1
@karlding
Copy link
Collaborator

This is fixed in develop inadvertently as a consequence of 6a38deb, which removed __getattr__/__setattr__, and self._dict from __slots__.

The problem is that __getattr__ incorrectly assumed that self._dict exists. Otherwise, if self._dict doesn't exist, it tried to call __getattr__ again, which results in the infinite recursion.

diff --git i/can/message.py w/can/message.py
index f85218f..1675e49 100644
--- i/can/message.py
+++ w/can/message.py
@@ -54,6 +54,8 @@ class Message(object):
         # this entire method (as well as the _dict attribute in __slots__ and the __setattr__ method)
         # can be removed in 4.0
         # this method is only called if the attribute was not found elsewhere, like in __slots__
+        if key not in self.__slots__:
+            raise AttributeError
         try:
             warnings.warn("Custom attributes of messages are deprecated and will be removed in 4.0", DeprecationWarning)
             return self._dict[key]

@hardbyte, would you prefer a fix similar to this diff, or should we just backport the removal from develop to the 3.3.3 branch?

@karlding karlding added the bug label Apr 20, 2020
karlding added a commit to karlding/python-can that referenced this issue Aug 5, 2020
__getattr__ incorrectly assumes that self._dict always exists. However,
if self._dict doesn't exist, the function attempts to call __getattr__
again, which results in infinite recursion when serializing the object
via pickle.

This fixes the implementation of __getattr__ and adds a test to exercise
pickling/unpickling the Message.

Fixes hardbyte#804
hardbyte pushed a commit that referenced this issue Aug 5, 2020
__getattr__ incorrectly assumes that self._dict always exists. However,
if self._dict doesn't exist, the function attempts to call __getattr__
again, which results in infinite recursion when serializing the object
via pickle.

This fixes the implementation of __getattr__ and adds a test to exercise
pickling/unpickling the Message.

Fixes #804
karlding added a commit to karlding/python-can that referenced this issue Aug 28, 2020
Add a test that exercises pickling/unpickling of a Message.

Addresses hardbyte#804
hardbyte pushed a commit that referenced this issue Aug 28, 2020
Add a test that exercises pickling/unpickling of a Message.

Addresses #804
@karlding karlding added this to the 3.3.4 Release milestone Sep 19, 2020
@karlding
Copy link
Collaborator

The change has been merged in the release-3.3.4 branch and the test has been cherry-picked to develop.

As such, closing this out. The fix will be available when 3.3.4 is released to PyPI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants