Skip to content

Faulty modules handling #261

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

Closed
pierreglaser opened this issue Apr 17, 2019 · 3 comments
Closed

Faulty modules handling #261

pierreglaser opened this issue Apr 17, 2019 · 3 comments

Comments

@pierreglaser
Copy link
Member

in #136, a test testing bad interactions happening in pickle.whichmodule was enhanced. But actually, it mangles 2 different use-cases into one:

The first use-case is calling pickle.whichmodule on an object with __module__ set to None. In this case, whichmodule loops through all imported modules, trying to find the object in each of them using the object's name.

This loop can error out if a potentially unrelated module throws an exceptions when someone tries to call getattr on it. Here is a reconstructed minimal example

import pickle
import sys


class UnrelatedTroublesomeModuleClass:
    def __getattr__(self, name):
        raise Exception

unrelated_troublesome_module = UnrelatedTroublesomeModuleClass()

# nothings prevents us from adding this instance to sys.modules
sys.modules['unrelated_troublesome_module'] = unrelated_troublesome_module

class A:
    def f(self):
        pass
    f.__module__ = None

# fails when looping over sys.modules and calling
# `` getattr(unrelated_troublesome_module, 'A')``
a = pickle.whichmodule(A, 'A')

The second use-case is when the object actually points to a faulty module, via its __module__ attribute. This module can for instance throw a Exception when calling gettattr on it. In this situation, exceptions can happen on pickle.whichmodule AND, further along the way during getattr(module, name) calls in save_function. Change the line f.__module__ = None to f.__module__ = 'unrelated_troublesome_module' to switch between the first and the second use-case.

2 remarks:

  • I think we want to support the use-case 1, meaning that we do not want unrelated faulty modules to cause failures when trying to infer the module of a given object
  • However, if this object explicitally points to a faulty module, I think we should not try/except the subsequent errors, but rather do not interfere. If the module throws an error when trying to call getattr, let it happen.

We may also want to implement our own whichmodule, that silently discards errors happening when calling getattr.

@ogrisel

@ogrisel
Copy link
Contributor

ogrisel commented Apr 17, 2019

I agree.

@ogrisel
Copy link
Contributor

ogrisel commented May 14, 2019

To be tackled once #262 is reviewed and merged.

@pierreglaser
Copy link
Member Author

Closing now that #273 was merged.

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

No branches or pull requests

2 participants