Skip to content

Commit

Permalink
fix missing is_dill guards from #463
Browse files Browse the repository at this point in the history
  • Loading branch information
mmckerns committed May 6, 2022
1 parent 6cafbf1 commit df6ab36
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions dill/_dill.py
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,7 @@ def _import_module(import_name, safe=False):

def _locate_function(obj, pickler=None):
if obj.__module__ in ['__main__', None] or \
pickler and pickler._session and obj.__module__ == pickler._main.__name__:
pickler and is_dill(pickler, child=False) and pickler._session and obj.__module__ == pickler._main.__name__:

This comment has been minimized.

Copy link
@leogama

leogama May 6, 2022

Contributor

I don't get the need of a safeguard here. In what kind of situation would the pickler not be dill.Pickler? In a subclass with a modified dump_session() that doesn't set all the attributes?

This comment has been minimized.

Copy link
@mmckerns

mmckerns May 6, 2022

Author Member

numpy, multiprocessing, and some other packages use their own picklers as part of the recursive pickle chain. So, I noticed it when theNumpyPickler complained when dill was trying to serialize a numpy ufunc. There's an example of this in the klepto test cases.

This comment has been minimized.

Copy link
@leogama

leogama May 6, 2022

Contributor

Interesting. I will do some tests to understand this. What I suppose is happening is:

dill calls @register(ufunc) -> dill.Pickler.dump(ufunc) -> pickle.Pickler.dump(ufunc) -> dill._dill.save_numpy_ufunc(pickler, ufunc) -> pickle.Pickler.save_global(pickler, ufunc) -> numpy hijacks the pickler somehow -> ... -> numpy_pickler.dump(ufunc_child_object) -> dill._dill.save_ufunc_child_object_type(numpy_pickler, ufunc_child_object) -> ... -> dill._dill._locate_function(obj, numpy_pickler) -> pickler AttributeError

This behavior can potentially circumvent dill's function namespace logic...

return False

found = _import_module(obj.__module__ + '.' + obj.__name__, safe=True)
Expand Down Expand Up @@ -1893,6 +1893,7 @@ def save_function(pickler, obj):
_byref = getattr(pickler, '_byref', None)
_postproc = getattr(pickler, '_postproc', None)
_main_modified = getattr(pickler, '_main_modified', None)
_original_main = getattr(pickler, '_original_main', __builtin__)#'None'
postproc_list = []
if _recurse:
# recurse to get all globals referred to by obj
Expand All @@ -1907,10 +1908,11 @@ def save_function(pickler, obj):
else:
globs_copy = obj.__globals__ if PY3 else obj.func_globals

# If the globals is the __dict__ from the module being save as a
# If the globals is the __dict__ from the module being saved as a
# session, substitute it by the dictionary being actually saved.
if _main_modified and globs_copy is pickler._original_main.__dict__:
globs = globs_copy = pickler._main.__dict__
if _main_modified and globs_copy is _original_main.__dict__:
globs_copy = getattr(pickler, '_main', _original_main).__dict__
globs = globs_copy
# If the globals is a module __dict__, do not save it in the pickle.
elif globs_copy is not None and obj.__module__ is not None and \
getattr(_import_module(obj.__module__, True), '__dict__', None) is globs_copy:
Expand Down

0 comments on commit df6ab36

Please sign in to comment.