Handle exceptions from dynamic_source_filename.

This required disabling plugins from the C tracer.

3 files changed, 106 insertions, 48 deletions

diff --git a/coverage/control.py b/coverage/control.py
index 2c0d9edc..563925ef 100644
--- a/coverage/control.py
+++ b/coverage/control.py
@@ -168,7 +168,7 @@ class Coverage(object):
         self.omit = self.include = self.source = None
         self.source_pkgs = self.file_locator = None
         self.data = self.collector = None
-        self.plugins = self.file_tracers = None
+        self.plugins = self.file_tracing_plugins = None
         self.pylib_dirs = self.cover_dir = None
         self.data_suffix = self.run_suffix = None
         self._exclude_re = None
@@ -203,10 +203,10 @@ class Coverage(object):
         # Load plugins
         self.plugins = Plugins.load_plugins(self.config.plugins, self.config)
 
-        self.file_tracers = []
+        self.file_tracing_plugins = []
         for plugin in self.plugins:
             if overrides(plugin, "file_tracer", CoveragePlugin):
-                self.file_tracers.append(plugin)
+                self.file_tracing_plugins.append(plugin)
 
         # _exclude_re is a dict that maps exclusion list names to compiled
         # regexes.
@@ -242,16 +242,17 @@ class Coverage(object):
             )
 
         # Early warning if we aren't going to be able to support plugins.
-        if self.file_tracers and not self.collector.supports_plugins:
+        if self.file_tracing_plugins and not self.collector.supports_plugins:
             self._warn(
                 "Plugin file tracers (%s) aren't supported with %s" % (
                     ", ".join(
-                        ft._coverage_plugin_name for ft in self.file_tracers
+                        plugin._coverage_plugin_name
+                            for plugin in self.file_tracing_plugins
                         ),
                     self.collector.tracer_name(),
                     )
                 )
-            for plugin in self.file_tracers:
+            for plugin in self.file_tracing_plugins:
                 plugin._coverage_enabled = False
 
         # Suffixes are a bit tricky.  We want to use the data suffix only when
@@ -464,15 +465,14 @@ class Coverage(object):
 
         # Try the plugins, see if they have an opinion about the file.
         plugin = None
-        for plugin in self.file_tracers:
+        for plugin in self.file_tracing_plugins:
             if not plugin._coverage_enabled:
                 continue
 
             try:
                 file_tracer = plugin.file_tracer(canonical)
                 if file_tracer is not None:
-                    file_tracer._coverage_plugin_name = \
-                        plugin._coverage_plugin_name
+                    file_tracer._coverage_plugin = plugin
                     disp.trace = True
                     disp.file_tracer = file_tracer
                     if file_tracer.has_dynamic_source_filename():
@@ -1019,12 +1019,12 @@ class Coverage(object):
         except AttributeError:
             implementation = "unknown"
 
-        file_tracers = []
-        for ft in self.file_tracers:
+        ft_plugins = []
+        for ft in self.file_tracing_plugins:
             ft_name = ft._coverage_plugin_name
             if not ft._coverage_enabled:
                 ft_name += " (disabled)"
-            file_tracers.append(ft_name)
+            ft_plugins.append(ft_name)
 
         info = [
             ('version', covmod.__version__),
@@ -1032,7 +1032,7 @@ class Coverage(object):
             ('cover_dir', self.cover_dir),
             ('pylib_dirs', self.pylib_dirs),
             ('tracer', self.collector.tracer_name()),
-            ('file_tracers', file_tracers),
+            ('file_tracing_plugins', ft_plugins),
             ('config_files', self.config.attempted_config_files),
             ('configs_read', self.config.config_files),
             ('data_path', self.data.filename),
diff --git a/coverage/tracer.c b/coverage/tracer.c
index 56703264..c63d3975 100644
--- a/coverage/tracer.c
+++ b/coverage/tracer.c
@@ -21,7 +21,9 @@
 
 #define MyText_Type         PyUnicode_Type
 #define MyText_AS_BYTES(o)  PyUnicode_AsASCIIString(o)
-#define MyText_AS_STRING(o) PyBytes_AS_STRING(o)
+#define MyBytes_AS_STRING(o) PyBytes_AS_STRING(o)
+#define MyText_AsString(o)  PyUnicode_AsUTF8(o)
+#define MyText_FromFormat   PyUnicode_FromFormat
 #define MyInt_FromInt(i)    PyLong_FromLong((long)i)
 #define MyInt_AsInt(o)      (int)PyLong_AsLong(o)
 
@@ -31,7 +33,9 @@
 
 #define MyText_Type         PyString_Type
 #define MyText_AS_BYTES(o)  (Py_INCREF(o), o)
-#define MyText_AS_STRING(o) PyString_AS_STRING(o)
+#define MyBytes_AS_STRING(o) PyString_AS_STRING(o)
+#define MyText_AsString(o)  PyString_AsString(o)
+#define MyText_FromFormat   PyUnicode_FromFormat
 #define MyInt_FromInt(i)    PyInt_FromLong((long)i)
 #define MyInt_AsInt(o)      (int)PyInt_AsLong(o)
 
@@ -298,7 +302,7 @@ showlog(int depth, int lineno, PyObject * filename, const char * msg)
         }
         if (filename) {
             PyObject *ascii = MyText_AS_BYTES(filename);
-            printf(" %s", MyText_AS_STRING(ascii));
+            printf(" %s", MyBytes_AS_STRING(ascii));
             Py_DECREF(ascii);
         }
         if (msg) {
@@ -457,7 +461,9 @@ CTracer_handle_call(CTracer *self, PyFrameObject *frame)
     PyObject * tracename = NULL;
     PyObject * disposition = NULL;
     PyObject * disp_trace = NULL;
-    PyObject * disp_file_tracer = NULL;
+    PyObject * file_tracer = NULL;
+    PyObject * plugin = NULL;
+    PyObject * plugin_name = NULL;
     PyObject * has_dynamic_filename = NULL;
 
     /* Borrowed references. */
@@ -507,10 +513,20 @@ CTracer_handle_call(CTracer *self, PyFrameObject *frame)
         if (tracename == NULL) {
             goto error;
         }
-        disp_file_tracer = PyObject_GetAttrString(disposition, "file_tracer");
-        if (disp_file_tracer == NULL) {
+        file_tracer = PyObject_GetAttrString(disposition, "file_tracer");
+        if (file_tracer == NULL) {
             goto error;
         }
+        if (file_tracer != Py_None) {
+            plugin = PyObject_GetAttrString(file_tracer, "_coverage_plugin");
+            if (plugin == NULL) {
+                goto error;
+            }
+            plugin_name = PyObject_GetAttrString(plugin, "_coverage_plugin_name");
+            if (plugin_name == NULL) {
+                goto error;
+            }
+        }
         has_dynamic_filename = PyObject_GetAttrString(disposition, "has_dynamic_filename");
         if (has_dynamic_filename == NULL) {
             goto error;
@@ -518,11 +534,42 @@ CTracer_handle_call(CTracer *self, PyFrameObject *frame)
         if (has_dynamic_filename == Py_True) {
             PyObject * next_tracename = NULL;
             next_tracename = PyObject_CallMethod(
-                disp_file_tracer, "dynamic_source_filename",
+                file_tracer, "dynamic_source_filename",
                 "OO", tracename, frame
                 );
             if (next_tracename == NULL) {
-                goto error;
+                /* An exception from the function. Alert the user with a
+                 * warning and a traceback.
+                 */
+                PyObject * ignored = NULL;
+                PyObject * msg = NULL;
+
+                msg = MyText_FromFormat(
+                    "Disabling plugin '%s' due to an exception:",
+                    MyText_AsString(plugin_name)
+                    );
+                if (msg == NULL) {
+                    goto error;
+                }
+                ignored = PyObject_CallFunctionObjArgs(self->warn, msg, NULL);
+                if (ignored == NULL) {
+                    goto error;
+                }
+                Py_DECREF(msg);
+                Py_DECREF(ignored);
+
+                PyErr_Print();
+
+                /* Disable the plugin for future files, and stop tracing this file. */
+                if (PyObject_SetAttrString(plugin, "_coverage_enabled", Py_False) < 0) {
+                    goto error;
+                }
+                if (PyObject_SetAttrString(disposition, "trace", Py_False) < 0) {
+                    goto error;
+                }
+
+                /* Because we handled the error, goto ok. */
+                goto ok;
             }
             Py_DECREF(tracename);
             tracename = next_tracename;
@@ -556,7 +603,6 @@ CTracer_handle_call(CTracer *self, PyFrameObject *frame)
 
     if (tracename != Py_None) {
         PyObject * file_data = PyDict_GetItem(self->data, tracename);
-        PyObject * disp_plugin_name = NULL;
 
         if (file_data == NULL) {
             file_data = PyDict_New();
@@ -570,13 +616,8 @@ CTracer_handle_call(CTracer *self, PyFrameObject *frame)
             }
 
             /* If the disposition mentions a plugin, record that. */
-            if (disp_file_tracer != Py_None) {
-                disp_plugin_name = PyObject_GetAttrString(disp_file_tracer, "_coverage_plugin_name");
-                if (disp_plugin_name == NULL) {
-                    goto error;
-                }
-                ret2 = PyDict_SetItem(self->plugin_data, tracename, disp_plugin_name);
-                Py_DECREF(disp_plugin_name);
+            if (file_tracer != Py_None) {
+                ret2 = PyDict_SetItem(self->plugin_data, tracename, plugin_name);
                 if (ret2 < 0) {
                     goto error;
                 }
@@ -584,7 +625,7 @@ CTracer_handle_call(CTracer *self, PyFrameObject *frame)
         }
 
         self->cur_entry.file_data = file_data;
-        self->cur_entry.file_tracer = disp_file_tracer;
+        self->cur_entry.file_tracer = file_tracer;
 
         /* Make the frame right in case settrace(gettrace()) happens. */
         Py_INCREF(self);
@@ -599,13 +640,16 @@ CTracer_handle_call(CTracer *self, PyFrameObject *frame)
 
     self->cur_entry.last_line = -1;
 
+ok:
     ret = RET_OK;
 
 error:
     Py_XDECREF(tracename);
     Py_XDECREF(disposition);
     Py_XDECREF(disp_trace);
-    Py_XDECREF(disp_file_tracer);
+    Py_XDECREF(file_tracer);
+    Py_XDECREF(plugin);
+    Py_XDECREF(plugin_name);
     Py_XDECREF(has_dynamic_filename);
 
     return ret;
@@ -742,14 +786,14 @@ CTracer_trace(CTracer *self, PyFrameObject *frame, int what, PyObject *arg_unuse
     #if WHAT_LOG
     if (what <= sizeof(what_sym)/sizeof(const char *)) {
         ascii = MyText_AS_BYTES(frame->f_code->co_filename);
-        printf("trace: %s @ %s %d\n", what_sym[what], MyText_AS_STRING(ascii), frame->f_lineno);
+        printf("trace: %s @ %s %d\n", what_sym[what], MyBytes_AS_STRING(ascii), frame->f_lineno);
         Py_DECREF(ascii);
     }
     #endif
 
     #if TRACE_LOG
     ascii = MyText_AS_BYTES(frame->f_code->co_filename);
-    if (strstr(MyText_AS_STRING(ascii), start_file) && frame->f_lineno == start_line) {
+    if (strstr(MyBytes_AS_STRING(ascii), start_file) && frame->f_lineno == start_line) {
         logging = 1;
     }
     Py_DECREF(ascii);
@@ -853,7 +897,7 @@ CTracer_call(CTracer *self, PyObject *args, PyObject *kwds)
        for the C function. */
     for (what = 0; what_names[what]; what++) {
         PyObject *ascii = MyText_AS_BYTES(what_str);
-        int should_break = !strcmp(MyText_AS_STRING(ascii), what_names[what]);
+        int should_break = !strcmp(MyBytes_AS_STRING(ascii), what_names[what]);
         Py_DECREF(ascii);
         if (should_break) {
             break;
diff --git a/tests/test_plugins.py b/tests/test_plugins.py
index 88c653c9..c8272514 100644
--- a/tests/test_plugins.py
+++ b/tests/test_plugins.py
@@ -442,15 +442,30 @@ class BadPluginTest(FileTracerTest):
     """Test error handling around plugins."""
 
     def run_bad_plugin(self, plugin_name, our_error=True):
-        """Run a file, and see that the plugin failed."""
+        """Run a file, and see that the plugin failed.
+
+        `plugin_name` is the name of the plugin to use.
+
+        `our_error` is True if the error reported to the user will be an
+        explicit error in our test code, marked with an # Oh noes! comment.
+
+        """
         self.make_file("simple.py", """\
-            import other
-            a = 2
-            b = 3
+            import other, another
+            a = other.f(2)
+            b = other.f(3)
+            c = another.g(4)
+            d = another.g(5)
             """)
+        # The names of these files are important: some plugins apply themselves
+        # to "*other.py".
         self.make_file("other.py", """\
-            x = 1
-            y = 2
+            def f(x):
+                return x+1
+            """)
+        self.make_file("another.py", """\
+            def g(x):
+                return x-1
             """)
 
         cov = coverage.Coverage()
@@ -466,9 +481,9 @@ class BadPluginTest(FileTracerTest):
             self.assertEqual(errors, 1)
 
         # There should be a warning explaining what's happening, but only one.
-        msg = "Disabling plugin '%s' due to an exception:" % plugin_name
-        tracebacks = stderr.count(msg)
-        self.assertEqual(tracebacks, 1)
+        msg = "Disabling plugin %r due to an exception:" % plugin_name
+        warnings = stderr.count(msg)
+        self.assertEqual(warnings, 1)
 
     def test_file_tracer_fails(self):
         self.make_file("bad_plugin.py", """\
@@ -527,19 +542,18 @@ class BadPluginTest(FileTracerTest):
             """)
         self.run_bad_plugin("bad_plugin", our_error=False)
 
-    # This test currently crashes the C tracer function.  I'm working on
-    # figuring out why....
-    def xxx_dynamic_source_filename_fails(self):
+    def test_dynamic_source_filename_fails(self):
         self.make_file("bad_plugin.py", """\
             import coverage.plugin
             class Plugin(coverage.plugin.CoveragePlugin):
                 def file_tracer(self, filename):
-                    return BadFileTracer()
+                    if filename.endswith("other.py"):
+                        return BadFileTracer()
 
             class BadFileTracer(coverage.plugin.FileTracer):
                 def has_dynamic_source_filename(self):
                     return True
                 def dynamic_source_filename(self, filename, frame):
-                    101/0
+                    101/0 # Oh noes!
             """)
         self.run_bad_plugin("bad_plugin")