sys.modules unpatching more trouble than it's worth

[wickman]

pex/pex.py has the following gem:

  # Thar be dragons -- when this contextmanager exits, the interpreter is
  # potentially in a wonky state since the patches here (minimum_sys_modules
  # for example) actually mutate global state.  This should not be
  # considered a reversible operation despite being a contextmanager.
  @classmethod
  @contextmanager
  def patch_sys(cls):
    """Patch sys with all site scrubbed."""
    def patch_dict(old_value, new_value):
      old_value.clear()
      old_value.update(new_value)

    def patch_all(path, path_importer_cache, modules):
      sys.path[:] = path
      patch_dict(sys.path_importer_cache, path_importer_cache)
      patch_dict(sys.modules, modules)

    old_sys_path, old_sys_path_importer_cache, old_sys_modules = (
        sys.path[:], sys.path_importer_cache.copy(), sys.modules.copy())
    new_sys_path, new_sys_path_importer_cache, new_sys_modules = cls.minimum_sys()

    patch_all(new_sys_path, new_sys_path_importer_cache, new_sys_modules)

    try:
      yield   
    finally:
      patch_all(old_sys_path, old_sys_path_importer_cache, old_sys_modules)

The first patch_all replaces the global sys.path, sys.path_importer_cache and sys.modules. This is fine, but the second patch_all replaces sys.modules with the original set, and thus de-refcounting all the modules that were imported during the lifecycle of the PEX. If you've registered sys.excepthook, atexit handlers or __del__, chances are you've seen inscrutable exceptions like 'NoneType' object has no attribute 'TBinaryProtocolAccelerated' or something. This is because of CPython interpreter teardown behavior: https://bugs.python.org/issue18214. This has since been "fixed" in CPython 3.4 but we can't mandate its use for everyone.

I'm proposing that we get rid of the unpatch. It is only used during PEX.execute, which behaves like "exec()" and should be the last call in any sensible program. I can't think of any reason to keep it other than some misguided sense of functional purity.

[jsirois]

+1