FIX always remove the callback from the callback itselffix-thread-safety

2 files changed, 16 insertions, 8 deletions

diff --git a/coverage/ctracer/tracer.c b/coverage/ctracer/tracer.c
index 236fbbf..0ade741 100644
--- a/coverage/ctracer/tracer.c
+++ b/coverage/ctracer/tracer.c
@@ -1035,7 +1035,10 @@ static PyObject *
 CTracer_stop(CTracer *self, PyObject *args_unused)
 {
     if (self->started) {
-        PyEval_SetTrace(NULL, NULL);
+        /* Set the started flag only. The actual call to
+           PyEval_SetTrace(NULL, NULL) is delegated to the callback
+           itself to ensure that it called from the right thread.
+           */
         self->started = FALSE;
     }
 
diff --git a/coverage/pytracer.py b/coverage/pytracer.py
index 6ebb60c..8f1b4b9 100644
--- a/coverage/pytracer.py
+++ b/coverage/pytracer.py
@@ -68,10 +68,10 @@ class PyTracer(object):
     def _trace(self, frame, event, arg_unused):
         """The trace function passed to sys.settrace."""
 
-        if self.stopped:
-            # The PyTrace.stop() method has been called by another thread,
-            # let's deactivate ourselves now.
-            self.stop()
+        if (self.stopped and sys.gettrace() == self._trace):
+            # The PyTrace.stop() method has been called, possibly by another
+            # thread, let's deactivate ourselves now.
+            sys.settrace(None)
             return
 
         if self.last_exc_back:
@@ -155,7 +155,15 @@ class PyTracer(object):
 
     def stop(self):
         """Stop this Tracer."""
+        # Get the activate tracer callback before setting the stop flag to be
+        # able to detect if the tracer was changed prior to stopping it.
+        tf = sys.gettrace()
+
+        # Set the stop flag. The actual call to sys.settrace(None) will happen
+        # in the self._trace callback itself to make sure to call it from the
+        # right thread.
         self.stopped = True
+
         if self.threading and self.thread.ident != self.threading.currentThread().ident:
             # Called on a different thread than started us: we can't unhook
             # ourselves, but we've set the flag that we should stop, so we
@@ -166,7 +174,6 @@ class PyTracer(object):
             # PyPy clears the trace function before running atexit functions,
             # so don't warn if we are in atexit on PyPy and the trace function
             # has changed to None.
-            tf = sys.gettrace()
             dont_warn = (env.PYPY and env.PYPYVERSION >= (5, 4) and self.in_atexit and tf is None)
             if (not dont_warn) and tf != self._trace:
                 self.warn(
@@ -174,8 +181,6 @@ class PyTracer(object):
                     slug="trace-changed",
                 )
 
-        sys.settrace(None)
-
     def activity(self):
         """Has there been any activity?"""
         return self._activity