From 72e355eaa67b255708030b80972242c93fdbae01 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
 <41898282+github-actions[bot]@users.noreply.github.com>
Date: Fri, 30 Aug 2024 16:15:14 +0000
Subject: [PATCH] fix: prevent module import callback side-effects [backport
 2.12] (#10319)

Backport 8b5fd7e5317df7bddd9c40f159ac71cfef4e17ba from #10316 to 2.12.

We make sure that we avoid side-effects when looking up methods from
module objects, as these could be of arbitrary type and have custom
attribute lookups implemented.

Addresses #10314.

## Testing Strategy

We try to replicate the side-effect caused by attribute lookup of the
custom module object in `yt-dlp`, and use that as a base for a
regression test.

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

Co-authored-by: Gabriele N. Tornetta <P403n1x87@users.noreply.github.com>
---
 ddtrace/internal/module.py                            |  7 +++++--
 ...ule-callback-no-side-effects-df422ae52b87772d.yaml |  5 +++++
 tests/internal/side_effect_module.py                  | 11 +++++++++++
 tests/internal/test_module.py                         |  7 +++++++
 4 files changed, 28 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/fix-module-callback-no-side-effects-df422ae52b87772d.yaml
 create mode 100644 tests/internal/side_effect_module.py

diff --git a/ddtrace/internal/module.py b/ddtrace/internal/module.py
index 26c12e66608..3c61a108ce3 100644
--- a/ddtrace/internal/module.py
+++ b/ddtrace/internal/module.py
@@ -201,7 +201,7 @@ def call_back(self, module: ModuleType) -> None:
         except (AttributeError, TypeError):
             pass
         try:
-            module.spec.loader = self.loader
+            object.__getattribute__(module, "spec").loader = self.loader
         except (AttributeError, TypeError):
             pass
 
@@ -313,7 +313,10 @@ def get_code(_loader, fullname):
                     exception_hook(self, module)
                 raise
 
-        self.call_back(module)
+        try:
+            self.call_back(module)
+        except Exception:
+            log.exception("Failed to call back on module %s", module)
 
 
 class BaseModuleWatchdog(abc.ABC):
diff --git a/releasenotes/notes/fix-module-callback-no-side-effects-df422ae52b87772d.yaml b/releasenotes/notes/fix-module-callback-no-side-effects-df422ae52b87772d.yaml
new file mode 100644
index 00000000000..bb656cbfc4c
--- /dev/null
+++ b/releasenotes/notes/fix-module-callback-no-side-effects-df422ae52b87772d.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fix for a side-effect issue with module import callbacks that could cause
+    a runtime exception.
diff --git a/tests/internal/side_effect_module.py b/tests/internal/side_effect_module.py
new file mode 100644
index 00000000000..5e1bd366371
--- /dev/null
+++ b/tests/internal/side_effect_module.py
@@ -0,0 +1,11 @@
+import sys
+from types import ModuleType
+
+
+class SideEffectModule(ModuleType):
+    def __getattribute__(self, name):
+        if name in {"spec"}:
+            raise RuntimeError("Attribute lookup side effect")
+
+
+sys.modules[__name__].__class__ = SideEffectModule
diff --git a/tests/internal/test_module.py b/tests/internal/test_module.py
index 7a3fd8de48b..daf2fb696b5 100644
--- a/tests/internal/test_module.py
+++ b/tests/internal/test_module.py
@@ -526,3 +526,10 @@ def test_module_watchdog_reloads_dont_cause_errors():
     sys.setrecursionlimit(1000)
     for _ in range(sys.getrecursionlimit() * 2):
         reload(ns_module)
+
+
+@pytest.mark.subprocess(ddtrace_run=True)
+def test_module_import_side_effect():
+    # Test that we can import a module that raises an exception during specific
+    # attribute lookups.
+    import tests.internal.side_effect_module  # noqa:F401