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

Fix RecursionError in Message.__getattr__ #885

Merged

Conversation

karlding
Copy link
Collaborator

@karlding karlding commented 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

__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
@karlding
Copy link
Collaborator Author

karlding commented Aug 5, 2020

I saw that there was a release-3.3.4 branch open... If that's still taking changes, would you consider merging this?

I'll cherry pick the test over to develop if you want the it there as well.

@hardbyte hardbyte merged commit 0c34e50 into hardbyte:release-3.3.4 Aug 5, 2020
@hardbyte
Copy link
Owner

hardbyte commented Aug 5, 2020

Regarding making this change in develop - there is a code comment saying this whole method can be removed in 4.0? Do you mind checking if that is the case and either patching/removing __getattr__ in dev.

cheers

@hardbyte hardbyte added this to the 3.3.4 Release milestone Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants