Skip to content

Commit 23e3215

Browse files
Yun-KimP403n1x87
andauthored
chore(internal): remove side-effects from origin [backport #4435 to 1.6] (#4811)
## Description Unblocks backport #4806. Accessing attributes from a module object in Python 2 seems to cause potential side-effects, like loading more modules. To prevent them, we ensure that we access attributes using the object type getter to bypass any potential methods that might trigger said side-effects. ``` > self._origin_map = {origin(module): module for module in sys.modules.values()} E RuntimeError: dictionary changed size during iteration ddtrace/internal/module.py:212: RuntimeError ``` ## Checklist - [ ] Add additional sections for `feat` and `fix` pull requests. - [ ] [Library documentation](https://github.com/DataDog/dd-trace-py/tree/1.x/docs) and/or [Datadog's documentation site](https://github.com/DataDog/documentation/) is updated. Link to doc PR in description. ## Reviewer Checklist - [x] Title is accurate. - [x] Description motivates each change. - [x] No unnecessary changes were introduced in this PR. - [x] Avoid breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Tests provided or description of manual testing performed is included in the code or PR. - [ ] Release note has been added for fixes and features, or else `changelog/no-changelog` label added. - [x] All relevant GitHub issues are correctly linked. - [x] Backports are identified and tagged with Mergifyio. ## Description <!-- Briefly describe the change and why it was required. --> <!-- If this is a breaking change, explain why it is necessary. Breaking changes must append `!` after the type/scope. See https://ddtrace.readthedocs.io/en/stable/contributing.html for more details. --> ## Checklist - [ ] Followed the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) when writing a release note. - [ ] Add additional sections for `feat` and `fix` pull requests. - [ ] [Library documentation](https://github.com/DataDog/dd-trace-py/tree/1.x/docs) and/or [Datadog's documentation site](https://github.com/DataDog/documentation/) is updated. Link to doc PR in description. <!-- Copy and paste the relevant snippet based on the type of pull request --> <!-- START feat --> ## Motivation <!-- Expand on why the change is required, include relevant context for reviewers --> ## Design <!-- Include benefits from the change as well as possible drawbacks and trade-offs --> ## Testing strategy <!-- Describe the automated tests and/or the steps for manual testing. <!-- END feat --> <!-- START fix --> ## Relevant issue(s) <!-- Link the pull request to any issues related to the fix. Use keywords for links to automate closing the issues once the pull request is merged. --> ## Testing strategy <!-- Describe any added regression tests and/or the manual testing performed. --> <!-- END fix --> ## Reviewer Checklist - [ ] Title is accurate. - [ ] Description motivates each change. - [ ] No unnecessary changes were introduced in this PR. - [ ] Avoid breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Tests provided or description of manual testing performed is included in the code or PR. - [ ] Release note has been added and follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines), or else `changelog/no-changelog` label added. - [ ] All relevant GitHub issues are correctly linked. - [ ] Backports are identified and tagged with Mergifyio. Co-authored-by: Gabriele N. Tornetta <[email protected]>
1 parent 0cad73a commit 23e3215

File tree

2 files changed

+5
-3
lines changed

2 files changed

+5
-3
lines changed

ddtrace/internal/module.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,14 @@ def origin(module):
9090
# type: (ModuleType) -> str
9191
"""Get the origin source file of the module."""
9292
try:
93-
orig = abspath(module.__file__) # type: ignore[type-var]
93+
# DEV: Use object.__getattribute__ to avoid potential side-effects.
94+
orig = abspath(object.__getattribute__(module, "__file__"))
9495
except (AttributeError, TypeError):
9596
# Module is probably only partially initialised, so we look at its
9697
# spec instead
9798
try:
98-
orig = abspath(module.__spec__.origin) # type: ignore
99+
# DEV: Use object.__getattribute__ to avoid potential side-effects.
100+
orig = abspath(object.__getattribute__(module, "__spec__").origin)
99101
except (AttributeError, ValueError, TypeError):
100102
orig = None
101103

tests/internal/test_module.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ def test_post_run_module_hook():
281281

282282

283283
def test_get_by_origin(module_watchdog):
284-
assert module_watchdog.get_by_origin(__file__) is sys.modules[__name__]
284+
assert module_watchdog.get_by_origin(__file__.replace(".pyc", ".py")) is sys.modules[__name__]
285285

286286

287287
@pytest.mark.subprocess

0 commit comments

Comments
 (0)