Skip to content

Patch cloudpickle to not reset dynamic class each time it is unpickled - #7388

Merged
sklam merged 7 commits into
numba:masterfrom
sklam:fix/iss7356
Sep 21, 2021
Merged

sklam merged 7 commits into
numba:masterfrom
sklam:fix/iss7356

Conversation

@sklam

@sklam sklam commented Sep 8, 2021

Copy link
Copy Markdown
Member

Fixes a problem with cloudpickle when dynamic class is unpickled and the class is reset to the state when it is pickled.

A short reproducer for the cloudpickle problem is available here:

# Tested with cloudpickle 1.6.0
from cloudpickle import dumps, loads


class Klass:
    classvar = None

def mutator():
    Klass.classvar = 100

def check():
    print("checking....")
    print(f"   Klass.classvar [{hex(id(Klass))}] = {Klass.classvar}")


def failing_case():
    print("Klass", hex(id(Klass)))
    saved = dumps(Klass)
    mutator()
    check()
    loads(saved)
    check()
    loads(saved)
    check()



if __name__ == '__main__':
    failing_case()

Note, cloudpickle treats any class created in __main__ as dynamic.

The patch makes cloudpickle remember whether a dynamic class is being reused. Reuse can come from the class being unpickled in the same process as it being pickled; or, a previously unpickled class is still around.

Reference an existing issue

Fixes #7356

@sklam
sklam marked this pull request as ready for review September 9, 2021 00:04
@sklam sklam added 3 - Ready for Review Effort - medium Medium size effort needed labels Sep 9, 2021
@sklam sklam added this to the Numba 0.54.1 milestone Sep 9, 2021

@stuartarchibald stuartarchibald left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch, a couple of minor things to resolve else looks good.

Comment thread numba/tests/test_serialize.py Outdated



class TestCloudPickleIssues(TestCase):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this ought to also include the original reproducer as a test? #7356 (comment)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Test added in 8f6ffdc

@stuartarchibald stuartarchibald added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Sep 17, 2021
sklam and others added 2 commits September 20, 2021 18:26
Co-authored-by: stuartarchibald <stuartarchibald@users.noreply.github.com>
@sklam sklam added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Sep 20, 2021

@stuartarchibald stuartarchibald left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch and fixes.

@stuartarchibald stuartarchibald added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Sep 21, 2021
@sklam
sklam merged commit ed6ae6d into numba:master Sep 21, 2021
@sklam
sklam deleted the fix/iss7356 branch September 21, 2021 22:33
sklam added a commit to sklam/numba that referenced this pull request Sep 22, 2021
Patch cloudpickle to not reset dynamic class each time it is unpickled
sklam added a commit to sklam/numba that referenced this pull request Oct 27, 2022
sklam added a commit to sklam/numba that referenced this pull request Nov 29, 2023
swap357 added a commit to swap357/numba that referenced this pull request Jul 15, 2025
swap357 added a commit to swap357/numba that referenced this pull request Jul 15, 2025
* upgrade cloudpickle to 3.1.1

* Adapt patch from numba#7388 to address dynamic class state reset issue numba#7356
esc added a commit to esc/numba that referenced this pull request Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

5 - Ready to merge Review and testing done, is ready to merge Effort - medium Medium size effort needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Numba 0.54.0: unexpected assignment behavior within objmode context.

2 participants