Skip to content

Commit

Permalink
Fix bug in optimizing module loader for coverage collection
Browse files Browse the repository at this point in the history
Summary:
In coverage collection mode a special module loader is prepended to
`sys.meta_path`. In very specific conditions this module loader can end up
returning a loader pointing to a _completely wrong module_. When importing
symbols from the wrong module errors occur.

The conditions to trigger the bug are:

- running in coverage collection mode, enabling the custom loader
- the test binary is a zip (e.g. par_style=fastzip)
- having a module name where the end part matches the name of a builtin Python
  module

When these conditions were met, the special loader would return the builtin
Python module instead of the expected module. E.g. when loading a module like
`myapp.somemod.platform` in a zip style binary.

The custom loader first calls `imp.find_module()` to find the module it wants
to return a wrapped loader for. This fails for modules included in the test
binary, because the builtin `imp` module can not load from zips. This was the
trigger leading to the call to the buggy code.

When the initial call to `imp.find_module()` failed, the custom loader would
try a second call, asking the internal loader to this time try any path on
`sys.path`. For most module names this call would also fail, making the custom
loader return `None`, after which Python tries other loaders on `sys.path`.
However, when the final part of the module that was asked to load matches the
name of a Python builtin module, then the second call to the `imp` module would
succeed, returning a loader for the builtin module. E.g. `platform` when asking
for `myapp.somemod.platform`.

This diff fixes the issue by removing the broken second call to the internal
loader. This will never have worked, we just have not triggered or noticed
triggering the wrong loading before.

Differential Revision: D20798119

fbshipit-source-id: dffb54e308106a81af21b63c5ee64c6ca2041920
  • Loading branch information
asm89 authored and facebook-github-bot committed Apr 2, 2020
1 parent 158e5c7 commit 792c0a7
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions build/fbcode_builder/CMake/fb_py_test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ def find_module(self, fullname, path=None):
try:
fd, pypath, (_, _, kind) = imp.find_module(basename, path)
except Exception:
# Maybe it's a top level module
try:
fd, pypath, (_, _, kind) = imp.find_module(basename, None)
except Exception:
return None
# Finding without hooks using the imp module failed. One reason
# could be that there is a zip file on sys.path. The imp module
# does not support loading from there. Leave finding this module to
# the others finders in sys.meta_path.
return None

if hasattr(fd, "close"):
fd.close()
Expand Down

0 comments on commit 792c0a7

Please sign in to comment.