Alternative fix for bug when pickling function from unloaded module

[leogama]

This patch modifies _locate_function() and save_function() to allow dill to save a function from a module that was unloaded, i.e. deleted from the sys.modules dictionary, by reference (if it would be saved as global had the module not been unloaded). Note: this is independent of the shared namespace problem.

It may be possible to do the same for classes and methods, but these cases are more complex.

Fixes: #532

[anivegesana]

I am confused about the purpose of caching the namespace. I have seen situations similar to #532 in popular packages (torch.package) but never for standard library modules, probably because you shouldn't change them, so there is no reason to reload them in the first place.

Plus, what if someone changes a file and calls importlib.reload to forcibly reload a package with new source code. The cached namespace wouldn't be updated and would cause confusion. Is there any reason that this should be included? If so, there should be a way to turn it off.

[leogama]

I am confused about the purpose of caching the namespace. I have seen situations similar to #532 in popular packages (torch.package) but never for standard library modules, probably because you shouldn't change them, so there is no reason to reload them in the first place.

I used a stdlib module for demonstration, but this mechanism would be triggered mainly by third-party packages, as in the example you mentioned. Reloaded modules are not covered by this patch, only "unloaded" ones. With a reloaded module, modified or not, dill (pickle) would still raise an "object is not the same as XXX.YYY" error.

Plus, what if someone changes a file and calls importlib.reload to forcibly reload a package with new source code. The cached namespace wouldn't be updated and would cause confusion.

This conflict would only happen with a series of events like:

import importlib
import sys
import dill
from module import func
dill.dumps(func)  # save as global
del sys.modules['module']
dill.dumps(func)  # add 'module' to _modules_copy's cache, save as global (this is the difference in behavior)
#--- Modify func in module.py ---#
import module  # re-import into sys.modules (creates a new module instance)
importlib.reload(module)  # required to update module.func?
dill.dumps(func)  # func is not module.func, raise error
del sys.modules['module']
dill.dumps(func)  # func is different from cached _modules_copy('module').func, save by value

In this last statement, the functions wouldn't match in the _locate_function() test and the logic would fall back to the old behavior (the function would be saved by value). The namespaces are cached because they are copies independent of the loaded modules and so that a new copy of the unloaded module isn't created for every object. Not caching would matter only in that case of "load -> import object -> unload -> pickle object -> reload with changes -> unload -> pickle object". The cache could be associated with the Pickler instance, although it wouldn't have effect for a sequence of calls to dill.dump[s] then.

Note: I have no preference for this or any of the other proposed solutions, I'm mostly throwing things against the wall.