Make World thread-safe

crates/typst-cli/src/fonts.rs

@@ -1,6 +1,6 @@
-use std::cell::OnceCell;
 use std::fs;
 use std::path::PathBuf;
+use std::sync::OnceLock;
 
 use fontdb::{Database, Source};
 use typst::diag::StrResult;
@@ -42,7 +42,7 @@ pub struct FontSlot {
     /// to a collection.
     index: u32,
     /// The lazily loaded font.
-    font: OnceCell<Option<Font>>,
+    font: OnceLock<Option<Font>>,
 }
 
 impl FontSlot {
@@ -92,7 +92,7 @@ impl FontSearcher {
                 self.fonts.push(FontSlot {
                     path: path.clone(),
                     index: face.index,
-                    font: OnceCell::new(),
+                    font: OnceLock::new(),
                 });
             }
         }
@@ -112,7 +112,7 @@ impl FontSearcher {
                 self.fonts.push(FontSlot {
                     path: PathBuf::new(),
                     index: i as u32,
-                    font: OnceCell::from(Some(font)),
+                    font: OnceLock::from(Some(font)),
                 });
             }
         };

crates/typst-cli/src/world.rs

@@ -1,6 +1,6 @@
-use std::cell::{OnceCell, RefCell, RefMut};
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
+use std::sync::{OnceLock, RwLock};
 use std::{fs, mem};
 
 use chrono::{DateTime, Datelike, Local};
@@ -34,10 +34,10 @@ pub struct SystemWorld {
     /// Locations of and storage for lazily loaded fonts.
     fonts: Vec<FontSlot>,
     /// Maps file ids to source files and buffers.
-    slots: RefCell<HashMap<FileId, FileSlot>>,
+    slots: RwLock<HashMap<FileId, FileSlot>>,
     /// The current datetime if requested. This is stored here to ensure it is
     /// always the same within one compilation. Reset between compilations.
-    now: OnceCell<DateTime<Local>>,
+    now: OnceLock<DateTime<Local>>,
     /// The export cache, used for caching output files in `typst watch`
     /// sessions.
     export_cache: ExportCache,
@@ -78,8 +78,8 @@ impl SystemWorld {
             library: Prehashed::new(Library::build()),
             book: Prehashed::new(searcher.book),
             fonts: searcher.fonts,
-            slots: RefCell::default(),
+            slots: RwLock::new(HashMap::new()),
-            now: OnceCell::new(),
+            now: OnceLock::new(),
             export_cache: ExportCache::new(),
         })
     }
@@ -103,6 +103,7 @@ impl SystemWorld {
     pub fn dependencies(&mut self) -> impl Iterator<Item = PathBuf> + '_ {
         self.slots
             .get_mut()
+            .unwrap()
             .values()
             .filter(|slot| slot.accessed())
             .filter_map(|slot| system_path(&self.root, slot.id).ok())
@@ -110,7 +111,7 @@ impl SystemWorld {
 
     /// Reset the compilation state in preparation of a new compilation.
     pub fn reset(&mut self) {
-        for slot in self.slots.get_mut().values_mut() {
+        for slot in self.slots.get_mut().unwrap().values_mut() {
             slot.reset();
         }
         self.now.take();
@@ -147,11 +148,11 @@ impl World for SystemWorld {
     }
 
     fn source(&self, id: FileId) -> FileResult<Source> {
-        self.slot(id)?.source(&self.root)
+        self.slot(id, |slot| slot.source(&self.root))
     }
 
     fn file(&self, id: FileId) -> FileResult<Bytes> {
-        self.slot(id)?.file(&self.root)
+        self.slot(id, |slot| slot.file(&self.root))
     }
 
     fn font(&self, index: usize) -> Option<Font> {
@@ -176,11 +177,12 @@ impl World for SystemWorld {
 
 impl SystemWorld {
     /// Access the canonical slot for the given file id.
-    #[tracing::instrument(skip_all)]
+    fn slot<F, T>(&self, id: FileId, f: F) -> T
-    fn slot(&self, id: FileId) -> FileResult<RefMut<FileSlot>> {
+    where
-        Ok(RefMut::map(self.slots.borrow_mut(), |slots| {
+        F: FnOnce(&mut FileSlot) -> T,
-            slots.entry(id).or_insert_with(|| FileSlot::new(id))
+    {
-        }))
+        let mut map = self.slots.write().unwrap();
+        f(map.entry(id).or_insert_with(|| FileSlot::new(id)))
     }
 }
 

crates/typst/src/engine.rs

@@ -1,4 +1,4 @@
-use std::cell::Cell;
+use std::sync::atomic::{AtomicUsize, Ordering};
 
 use comemo::{Track, Tracked, TrackedMut, Validate};
 
@@ -24,7 +24,7 @@ pub struct Engine<'a> {
 }
 
 impl Engine<'_> {
-    /// Perform a fallible operation that does not immediately terminate further
+    /// Performs a fallible operation that does not immediately terminate further
     /// execution. Instead it produces a delayed error that is only promoted to
     /// a fatal one if it remains at the end of the introspection loop.
     pub fn delayed<F, T>(&mut self, f: F) -> T
@@ -44,7 +44,6 @@ impl Engine<'_> {
 
 /// The route the engine took during compilation. This is used to detect
 /// cyclic imports and too much nesting.
-#[derive(Clone)]
 pub struct Route<'a> {
     // We need to override the constraint's lifetime here so that `Tracked` is
     // covariant over the constraint. If it becomes invariant, we're in for a
@@ -63,7 +62,7 @@ pub struct Route<'a> {
     /// know the exact length (that would defeat the whole purpose because it
     /// would prevent cache reuse of some computation at different,
     /// non-exceeding depths).
-    upper: Cell<usize>,
+    upper: AtomicUsize,
 }
 
 /// The maximum nesting depths. They are different so that even if show rule and
@@ -84,7 +83,12 @@ impl Route<'_> {
 impl<'a> Route<'a> {
     /// Create a new, empty route.
     pub fn root() -> Self {
-        Self { id: None, outer: None, len: 0, upper: Cell::new(0) }
+        Self {
+            id: None,
+            outer: None,
+            len: 0,
+            upper: AtomicUsize::new(0),
+        }
     }
 
     /// Extend the route with another segment with a default length of 1.
@@ -93,7 +97,7 @@ impl<'a> Route<'a> {
             outer: Some(outer),
             id: None,
             len: 1,
-            upper: Cell::new(usize::MAX),
+            upper: AtomicUsize::new(usize::MAX),
         }
     }
 
@@ -138,7 +142,10 @@ impl<'a> Route<'a> {
 
     /// Whether the route's depth is less than or equal to the given depth.
     pub fn within(&self, depth: usize) -> bool {
-        if self.upper.get().saturating_add(self.len) <= depth {
+        use Ordering::Relaxed;
+
+        let upper = self.upper.load(Relaxed);
+        if upper.saturating_add(self.len) <= depth {
             return true;
         }
 
@@ -146,8 +153,10 @@ impl<'a> Route<'a> {
             Some(_) if depth < self.len => false,
             Some(outer) => {
                 let within = outer.within(depth - self.len);
-                if within && depth < self.upper.get() {
+                if within && depth < upper {
-                    self.upper.set(depth);
+                    // We don't want to accidentally increase the upper bound,
+                    // hence the compare-exchange.
+                    self.upper.compare_exchange(upper, depth, Relaxed, Relaxed).ok();
                 }
                 within
             }
@@ -161,3 +170,16 @@ impl Default for Route<'_> {
         Self::root()
     }
 }
+
+impl Clone for Route<'_> {
+    fn clone(&self) -> Self {
+        Self {
+            outer: self.outer,
+            id: self.id,
+            len: self.len,
+            // The ordering doesn't really matter since it's the upper bound
+            // is only an optimization.
+            upper: AtomicUsize::new(self.upper.load(Ordering::Relaxed)),
+        }
+    }
+}

tests/src/tests.rs

@@ -1,12 +1,12 @@
 #![allow(clippy::comparison_chain)]
 
-use std::cell::{RefCell, RefMut};
 use std::collections::{HashMap, HashSet};
 use std::ffi::OsStr;
 use std::fmt::{self, Display, Formatter, Write as _};
 use std::io::{self, IsTerminal, Write};
 use std::ops::Range;
 use std::path::{Path, PathBuf, MAIN_SEPARATOR_STR};
+use std::sync::{OnceLock, RwLock};
 use std::{env, fs};
 
 use clap::Parser;
@@ -14,7 +14,6 @@ use comemo::{Prehashed, Track};
 use ecow::EcoString;
 use oxipng::{InFile, Options, OutFile};
 use rayon::iter::{ParallelBridge, ParallelIterator};
-use std::cell::OnceCell;
 use tiny_skia as sk;
 use typst::diag::{bail, FileError, FileResult, Severity, StrResult};
 use typst::eval::Tracer;
@@ -217,20 +216,19 @@ fn library() -> Library {
 }
 
 /// A world that provides access to the tests environment.
-#[derive(Clone)]
 struct TestWorld {
     print: PrintConfig,
     main: FileId,
     library: Prehashed<Library>,
     book: Prehashed<FontBook>,
     fonts: Vec<Font>,
-    paths: RefCell<HashMap<FileId, PathSlot>>,
+    slots: RwLock<HashMap<FileId, FileSlot>>,
 }
 
 #[derive(Clone)]
-struct PathSlot {
+struct FileSlot {
-    source: OnceCell<FileResult<Source>>,
+    source: OnceLock<FileResult<Source>>,
-    buffer: OnceCell<FileResult<Bytes>>,
+    buffer: OnceLock<FileResult<Bytes>>,
 }
 
 impl TestWorld {
@@ -253,7 +251,7 @@ impl TestWorld {
             library: Prehashed::new(library()),
             book: Prehashed::new(FontBook::from_fonts(&fonts)),
             fonts,
-            paths: RefCell::default(),
+            slots: RwLock::new(HashMap::new()),
         }
     }
 }
@@ -272,7 +270,7 @@ impl World for TestWorld {
     }
 
     fn source(&self, id: FileId) -> FileResult<Source> {
-        let slot = self.slot(id)?;
+        self.slot(id, |slot| {
             slot.source
                 .get_or_init(|| {
                     let buf = read(&system_path(id)?)?;
@@ -280,13 +278,15 @@ impl World for TestWorld {
                     Ok(Source::new(id, text))
                 })
                 .clone()
+        })
     }
 
     fn file(&self, id: FileId) -> FileResult<Bytes> {
-        let slot = self.slot(id)?;
+        self.slot(id, |slot| {
             slot.buffer
                 .get_or_init(|| read(&system_path(id)?).map(Bytes::from))
                 .clone()
+        })
     }
 
     fn font(&self, id: usize) -> Option<Font> {
@@ -301,22 +301,37 @@ impl World for TestWorld {
 impl TestWorld {
     fn set(&mut self, path: &Path, text: String) -> Source {
         self.main = FileId::new(None, VirtualPath::new(path));
-        let mut slot = self.slot(self.main).unwrap();
         let source = Source::new(self.main, text);
-        slot.source = OnceCell::from(Ok(source.clone()));
+        self.slot(self.main, |slot| {
+            slot.source = OnceLock::from(Ok(source.clone()));
             source
+        })
     }
 
-    fn slot(&self, id: FileId) -> FileResult<RefMut<PathSlot>> {
+    fn slot<F, T>(&self, id: FileId, f: F) -> T
-        Ok(RefMut::map(self.paths.borrow_mut(), |paths| {
+    where
-            paths.entry(id).or_insert_with(|| PathSlot {
+        F: FnOnce(&mut FileSlot) -> T,
-                source: OnceCell::new(),
+    {
-                buffer: OnceCell::new(),
+        f(self.slots.write().unwrap().entry(id).or_insert_with(|| FileSlot {
-            })
+            source: OnceLock::new(),
+            buffer: OnceLock::new(),
         }))
     }
 }
 
+impl Clone for TestWorld {
+    fn clone(&self) -> Self {
+        Self {
+            print: self.print,
+            main: self.main,
+            library: self.library.clone(),
+            book: self.book.clone(),
+            fonts: self.fonts.clone(),
+            slots: RwLock::new(self.slots.read().unwrap().clone()),
+        }
+    }
+}
+
 /// The file system path for a file ID.
 fn system_path(id: FileId) -> FileResult<PathBuf> {
     let root: PathBuf = match id.package() {