fix: re-read files from disk on every compilation - #39
Open
msallin wants to merge 1 commit into
Open
Conversation
msallin
force-pushed
the
fix/reset-file-slots
branch
from
September 6, 2026 16:49
c0cb667 to
f92ccf1
Compare
A compiler filled its file slots on first access and never invalidated them, so the main file and every import stayed pinned to whatever they contained during the first compilation. Keeping one compiler alive across renders is the recommended way to use the library, and a template redeployed underneath a running process went on rendering the retired content with no error to show for it. Every compilation now starts by marking the slots as unread, the way typst-cli does between watch runs. A file whose content has not changed is still recognised by its hash and is not parsed again, and a document handed over as a string keeps its content: there is no file behind it to read. The reset takes the slot mutex rather than reaching past it with get_mut. The map is grown by every first access to a file, so iterating it unlocked would turn the documented one-compiler-per-thread rule from a stale-read hazard into a dangling iterator. Also wraps the calls in tests/input_path.rs in unsafe blocks. The exported functions became unsafe after those tests were written, so the test crate no longer compiled and the native test step failed.
msallin
force-pushed
the
fix/reset-file-slots
branch
from
September 6, 2026 17:14
f92ccf1 to
04780a4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #28.
Problem
SlotCell::accessedwas set on first access and never cleared, so a compiler kept its main file and every import pinned to whatever they contained during the first compilation. Rewriting a.typon disk and compiling again returned the old output, with no error.Caching a compiler is what the README recommends, so anyone who follows that advice and also redeploys templates gets silently stale documents.
Change
SystemWorld::reset(previouslyreset_time) now marks every file slot as not yet accessed before each compilation, the waytypst-clidoes between watch runs, in addition to clearing the cached date/time.SlotCellfilled byinit_in_memoryis pinned.TypstCompiler.FromSourcehas no file behind<main>, so invalidating it would make the second compilation fail. The exemption is per cell, not per file slot, so a document passed as a string still re-reads its imports..typ— which is the case this fixes. Same reasoning for retrying a package that failed to resolve.Mutex::get_mut. Every first access to a file inserts into that map and can reallocate it, so iterating it unlocked would turn the documented one-compiler-per-thread rule from a stale-read hazard into a dangling iterator.Consequences worth a decision
Both are documented in the README and the release notes, but they are behaviour changes rather than pure fixes:
Templates must be replaced atomically. A compilation landing halfway through a plain overwrite renders the half-written file. Before, that window was unreachable for a warm compiler; now it is open on every request. Write-temp-then-rename is the fix on the deploying side.
A warm compile now costs one stat and one read per file it touches, and scales with file count. Measured on Windows (release build, warm page cache, 200 compilations of one compiler), per compilation:
Roughly 99 µs per file per compilation, all of it the forced read — an isolated benchmark of
fs::metadata+fs::read+hash128over the same files gives 97.4 µs. That figure is inflated by on-access AV scanning and would be much lower on Linux, but against the README's 3 ms warm compile it is not nothing for a template split across many files.Correctness by default seemed the right trade given the issue, but if you would rather have an opt-out, an FFI
invalidate_files(compiler)plus a managedInvalidateFiles()would let a server with its own deploy hook pay only when something actually changed — closer to whattypst-clidoes, since its resets are driven by a watcher rather than by each render. Happy to add it here or as a follow-up.Unrelated build fix
tests/input_path.rsdid not compile ondevelop: the exported functions becameunsafein #36, after those tests were written in #37, socargo testfailed with sevenE0133errors and the native test step inpack.ymlhas been red. The calls are now wrapped inunsafeblocks. Worth a look at why that went unnoticed.Tests
tests/file_reload.rs(8 tests): rewritten main file, rewritten import, rewritten data file read throughread(), a file repaired on disk after a failed compilation, a deleted file reported as missing, an in-memory document compiled twice, an in-memory document whose import is rewritten, and an in-memory source shadowing a file it names. Six of the eight fail ondevelop; the two in-memory ones guard against the reset going too far.world.rsgained white-box tests forSlotCellitself: that a reset sends the cell back to the file but decodes only what changed, that a file which did not exist yet is picked up, and that an in-memory value is left alone.Tests.cscovers the end-to-end cases: a template, an import, and a vendored local package rewritten underneath a reusedTypstCompiler.cargo testanddotnet testboth pass: 29 native, 59 managed, no warnings.Follow-ups I did not fold in
slot()holds the mutex acrossread,decode_utf8and package resolution, so a panic in any of them poisons it and every later call fails with aPoisonErrorthat masks the real error.get_or_initis self-healing after such a panic, sounwrap_or_else(PoisonError::into_inner)is the right recovery — but that is a pre-existing bug inslot(), not something this change introduces.compile.rs,input_path.rsandfile_reload.rs; atests/common/mod.rswould be worth it.README.mdrecommends caching a compiler in ASP.NET whilelib.rsdocuments that one compiler must not be used from two threads, andTypstCompilerhas no synchronisation. The two should be reconciled.