From 4b48a1459e070288465805ea26f50664a4b70567 Mon Sep 17 00:00:00 2001 From: Jack Yeh Date: Tue, 25 Aug 2026 13:56:13 +0800 Subject: [PATCH] fix: don't stop thread managers owned by another process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ThreadManager.is_started()` compares the pid that started the manager's threads with the current pid, but nothing ever called it. `Client.close()` stopped every thread manager unconditionally, so a pre-forked worker would cancel the timers its parent started. `Transport.close()` already guards on `self._thread.pid != os.getpid()`, so transport was safe; `MetricsRegistry` (guarded on `is_alive()`) and `VersionedConfig` (guarded on the attribute merely being set) were not. Servers that fork workers without running CPython's after-fork cleanup — uWSGI in its default prefork mode, i.e. without `--py-call-uwsgi-fork-hooks` — leave the child's `threading` module believing the parent's threads are still alive, so `is_alive()` stays True in the child and the guard does not help. Cancelling those timers calls `Event.set()` on objects belonging to threads that do not exist in this process. On Python 3.13+, where locks are built on `PyMutex`, that segfaults the worker in `_PyParkingLot_Unpark`. Guard the loop in `Client.close()` with the check the base class already provides, so it covers every thread manager instead of each one having to remember. Co-Authored-By: Claude Opus 5 --- elasticapm/base.py | 7 +++++++ tests/client/client_tests.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/elasticapm/base.py b/elasticapm/base.py index 2c82f0d88..bcca05086 100644 --- a/elasticapm/base.py +++ b/elasticapm/base.py @@ -336,6 +336,13 @@ def close(self) -> None: if self.config.enabled: with self._thread_starter_lock: for _, manager in sorted(self._thread_managers.items(), key=lambda item: item[1].start_stop_order): + if not manager.is_started(): + # The threads belong to another process: we were inherited by a + # pre-forked worker that has not started its own threads yet. + # Stopping them from here touches objects this process does not + # own, and on Python 3.13+ that can crash the interpreter. + self.logger.debug("Not stopping threads started by pid %r", manager.pid) + continue manager.stop_thread() global CLIENT_SINGLETON CLIENT_SINGLETON = None diff --git a/tests/client/client_tests.py b/tests/client/client_tests.py index e42ada12c..d493c9f49 100644 --- a/tests/client/client_tests.py +++ b/tests/client/client_tests.py @@ -527,6 +527,35 @@ def test_client_enabled(elasticapm_client): assert not manager.is_started() +@pytest.mark.parametrize( + "elasticapm_client", + [{"enabled": True, "metrics_interval": "30s", "central_config": "true"}], + indirect=True, +) +def test_close_leaves_threads_of_another_process_alone(elasticapm_client): + """ + A pre-forking server hands the worker a client whose thread managers still point at + the threads the parent started. Those threads do not exist in the worker, so close() + must leave them alone instead of cancelling them. + """ + metrics = elasticapm_client._thread_managers["metrics"] + config = elasticapm_client._thread_managers["config"] + collect_timer, update_thread = metrics._collect_timer, config._update_thread + assert collect_timer is not None and update_thread is not None + + for manager in elasticapm_client._thread_managers.values(): + assert manager.is_started() + manager.pid = os.getpid() + 1 + + try: + elasticapm_client.close() + assert metrics._collect_timer is collect_timer + assert config._update_thread is update_thread + finally: + for manager in elasticapm_client._thread_managers.values(): + manager.pid = os.getpid() + + def test_excepthook(elasticapm_client): try: raise Exception("hi!")