Skip to content

Make Documents Send and Sync - #161

Open
KayJay7 wants to merge 2 commits into
Paligo:mainfrom
esperto-input:threadsafe-documents
Open

Make Documents Send and Sync#161
KayJay7 wants to merge 2 commits into
Paligo:mainfrom
esperto-input:threadsafe-documents

Conversation

@KayJay7

@KayJay7 KayJay7 commented Aug 21, 2026

Copy link
Copy Markdown

The purpose of this is to allow sharing an xee_xpath::documents::Documents object across threads, so it can be queried and interacted with. This approach has some key differences from #138 that we will discuss.

The problem is that Documents contains a DocumentsRef, which is neither Send nor Sync. This pr replaces that DocumentsRef for a plain xee_interpreter::xml::Documents, and changes the DocumentsRef from an Rc<RefCell<xml::Documents>> into a simpler RefCell<&'a mut xml::Documents>. This allows us to "create" a DocumentsRef when it's time to pass it to the interpreter, and drop it soon after (which can't be done with Rcs), and tracking it's lifetime across the interpreter instead of relying on reference counting.

A consequence of this is that when you take a reference to the xml::Documents out of Documents it has a lifetime and is borrowchecked (e.g. mutably accessing Documents would invalidate the reference), so we had to make a small change to the API. We replaced Query::execute_with_context gets with Query::execute_with_builder, this way, the full context (with the xml::Documents reference) is only built when executing, and the builder doesn't need lifetime tracking.

Due to the size of this project, I had the help of AI for the grunt work of the implementation. However, every change has been checked and most of them are just adding lifetimes to signatures. cargo test passes all tests, and xee-testrunner passes and fails the same tests as before the PR.

Difference from #138

In #138, the DocumentsRef is made shareable between threads by replacing Rc<RefCell<xml::Documents>> with Arc<RwLock<xml::Documents>>. This leaves the API unchanged because it doesn't affects lifetimes, but has some major drawbacks:

  • Performance
    • While Arc is almost free, RwLock is not, I haven't tested but it's sure to have some impact on performance because the interpreter locks and unlocks it repeatedly instead of only at the start and end of execution.
  • Correctness
    • Now that DocumentsRef can be shared among threads, the compiler will allow running queries concurrently as long as you provide the Xot and the DynamicContext. But opening and closing the locks immediately only prevent the actual writes to overstep each other, but does not protect the two executions as a whole
  • Thread safety
    • Other than correctness, the interpreter was not designed with that in mind, so there's an high risk of deadlocks if multiple queries are run at the same time

KayJay7 and others added 2 commits August 21, 2026 18:14
Instead of storing a xee_interpreter::xml::DocumentsRef, Documents now stores a plain xee_interpreter::xml::Documents.
Without the Rc, xee_xpath::documents::Documents becomes send.
For this Query::execute_with_context gets replaced by Query::execute_with_builder.
…ync (Documents is also Sync)

Removing the RefCells in DocumentOrderAnnotations makes the struct Sync.
This makes both xee_interpreter::xml::Documents and xee_xpath::documents::Documents also Sync
Now xee_xpath::documents::Documents can be shared across threads
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant