Add a circuit breaker for #374 (#398)

crates/rune/src/compile/mod.rs

@@ -127,6 +127,8 @@ pub(crate) fn compile(
 
     loop {
         while let Some(entry) = worker.q.next_build_entry() {
+            tracing::trace!("next build entry: {}", entry.item.item);
+
             let source_id = entry.location.source_id;
 
             let task = CompileBuildEntry {
@@ -184,6 +186,7 @@ impl CompileBuildEntry<'_> {
         }
     }
 
+    #[tracing::instrument(skip(self, entry))]
     fn compile(mut self, entry: BuildEntry) -> Result<(), CompileError> {
         let BuildEntry {
             item,
@@ -196,6 +199,8 @@ impl CompileBuildEntry<'_> {
 
         match build {
             Build::Function(f) => {
+                tracing::trace!("function: {}", item.item);
+
                 use self::v1::assemble;
 
                 let args =
@@ -221,6 +226,8 @@ impl CompileBuildEntry<'_> {
                 }
             }
             Build::InstanceFunction(f) => {
+                tracing::trace!("instance function: {}", item.item);
+
                 use self::v1::assemble;
 
                 let args =
@@ -256,6 +263,8 @@ impl CompileBuildEntry<'_> {
                 }
             }
             Build::Closure(closure) => {
+                tracing::trace!("closure: {}", item.item);
+
                 use self::v1::assemble;
 
                 let span = closure.ast.span();
@@ -283,6 +292,8 @@ impl CompileBuildEntry<'_> {
                 }
             }
             Build::AsyncBlock(b) => {
+                tracing::trace!("async block: {}", item.item);
+
                 use self::v1::assemble;
 
                 let args = b.captures.len();
@@ -306,12 +317,16 @@ impl CompileBuildEntry<'_> {
                 }
             }
             Build::Unused => {
+                tracing::trace!("unused: {}", item.item);
+
                 if !item.visibility.is_public() {
                     self.diagnostics
                         .not_used(location.source_id, location.span, None);
                 }
             }
             Build::Import(import) => {
+                tracing::trace!("import: {}", item.item);
+
                 // Issue the import to check access.
                 let result = self
                     .q
@@ -341,6 +356,8 @@ impl CompileBuildEntry<'_> {
                 }
             }
             Build::ReExport => {
+                tracing::trace!("re-export: {}", item.item);
+
                 let import = match self
                     .q
                     .import(location.span, &item.module, &item.item, used)?

crates/rune/src/diagnostics/emit_diagnostics.rs

@@ -496,22 +496,10 @@ where
                 format_ir_error(this, sources, error_span, error, labels, notes)?;
             }
             QueryErrorKind::ImportCycle { path } => {
-                let mut it = path.iter();
-                let last = it.next_back();
-
-                for (step, entry) in (1..).zip(it) {
-                    labels.push(
-                        d::Label::secondary(entry.location.source_id, entry.location.span.range())
-                            .with_message(format!("step #{} for `{}`", step, entry.item)),
-                    );
-                }
-
-                if let Some(entry) = last {
-                    labels.push(
-                        d::Label::secondary(entry.location.source_id, entry.location.span.range())
-                            .with_message(format!("final step cycling back to `{}`", entry.item)),
-                    );
-                }
+                diagnose_import_path(&mut labels, path);
+            }
+            QueyrErrorKind::ImportRecursionLimit { path, .. } => {
+                diagnose_import_path(&mut labels, path);
             }
             QueryErrorKind::ItemConflict {
                 other: Location { source_id, span },
@@ -594,6 +582,28 @@ where
     ) -> fmt::Result {
         Ok(())
     }
+
+    fn diagnose_import_path(
+        labels: &mut Vec<d::Label<SourceId>>,
+        path: &[ImportStep],
+    ) {
+        let mut it = path.iter();
+        let last = it.next_back();
+
+        for (step, entry) in (1..).zip(it) {
+            labels.push(
+                d::Label::secondary(entry.location.source_id, entry.location.span.range())
+                    .with_message(format!("step #{} for `{}`", step, entry.item)),
+            );
+        }
+
+        if let Some(entry) = last {
+            labels.push(
+                d::Label::secondary(entry.location.source_id, entry.location.span.range())
+                    .with_message(format!("final step cycling back to `{}`", entry.item)),
+            );
+        }
+    }
 }
 
 impl EmitDiagnostics for FatalDiagnostic {

crates/rune/src/indexing/index.rs

@@ -1132,11 +1132,11 @@ fn expr(ast: &mut ast::Expr, idx: &mut Indexer<'_>, is_used: IsUsed) -> CompileR
         // NB: macros have nothing to index, they don't export language
         // items.
         ast::Expr::MacroCall(macro_call) => {
-            // Note: There is a preprocessing step involved with statemetns
-            // for which the macro **might** have been expanded to a
-            // built-in macro if we end up here. So instead of expanding if
-            // the id is set, we just assert that the builtin macro has been
-            // added to the query engine.
+            // Note: There is a preprocessing step involved with statements for
+            // which the macro **might** have been expanded to a built-in macro
+            // if we end up here. So instead of expanding if the id is set, we
+            // just assert that the builtin macro has been added to the query
+            // engine.
 
             if !macro_call.id.is_set() {
                 if !idx.try_expand_internal_macro(&mut attributes, macro_call)? {

crates/rune/src/query/mod.rs

@@ -22,6 +22,9 @@ use std::fmt;
 use std::num::NonZeroUsize;
 use std::sync::Arc;
 
+/// The permitted number of import recursions when constructing a path.
+const IMPORT_RECURSION_LIMIT: usize = 128;
+
 pub use self::query_error::{QueryError, QueryErrorKind};
 
 mod query_error;
@@ -808,6 +811,7 @@ impl<'a> Query<'a> {
     }
 
     /// Get the given import by name.
+    #[tracing::instrument(skip(self, span, module))]
     pub(crate) fn import(
         &mut self,
         span: Span,
@@ -821,7 +825,18 @@ impl<'a> Query<'a> {
         let mut item = item.clone();
         let mut any_matched = false;
 
+        let mut count = 0usize;
+
         'outer: loop {
+            if count > IMPORT_RECURSION_LIMIT {
+                return Err(QueryError::new(
+                    span,
+                    QueryErrorKind::ImportRecursionLimit { count, path },
+                ));
+            }
+
+            count += 1;
+
             let mut cur = Item::new();
             let mut it = item.iter();
 
@@ -861,6 +876,7 @@ impl<'a> Query<'a> {
     }
 
     /// Inner import implementation that doesn't walk the imported name.
+    #[tracing::instrument(skip(self, span, module, path))]
     fn import_step(
         &mut self,
         span: Span,

crates/rune/src/query/query_error.rs

@@ -75,6 +75,8 @@ pub enum QueryErrorKind {
     MissingMod { item: Item },
     #[error("cycle in import")]
     ImportCycle { path: Vec<ImportStep> },
+    #[error("import recursion limit reached ({count})")]
+    ImportRecursionLimit { count: usize, path: Vec<ImportStep> },
     #[error("missing last use component")]
     LastUseComponent,
     #[error("found indexed entry for `{item}`, but was not an import")]

crates/rune/src/worker/import.rs

@@ -75,6 +75,8 @@ impl Import {
         queue.push_back((&self.ast.path, name, first, initial));
 
         while let Some((path, mut name, first, mut initial)) = queue.pop_front() {
+            tracing::trace!("process one");
+
             let mut it = first
                 .into_iter()
                 .chain(path.segments.iter().map(|(_, s)| s));

crates/rune/src/worker/mod.rs

@@ -103,7 +103,7 @@ impl<'a> Worker<'a> {
                         LoadFileKind::Module { root } => root,
                     };
 
-                    tracing::trace!("index: {}", mod_item.item);
+                    tracing::trace!("load file: {}", mod_item.item);
                     let items = Items::new(mod_item.item.clone(), self.gen);
 
                     let mut indexer = Indexer {
@@ -128,6 +128,8 @@ impl<'a> Worker<'a> {
                     }
                 }
                 Task::ExpandImport(import) => {
+                    tracing::trace!("expand import");
+
                     let source_id = import.source_id;
                     let queue = &mut self.queue;
 
@@ -140,6 +142,8 @@ impl<'a> Worker<'a> {
                     }
                 }
                 Task::ExpandWildcardImport(wildcard_import) => {
+                    tracing::trace!("expand wildcard import");
+
                     wildcard_imports.push(wildcard_import);
                 }
             }