-
Notifications
You must be signed in to change notification settings - Fork 0
Concurrency
Toolkit: Send + !Sync.
Verovio's render and layout methods (RenderToSVG, GetPageCount,
GetElementsAtTime, RedoLayout, …) are non-const upstream — they
mutate internal state even when shaped as read calls. With cxx, these
take Pin<&mut Toolkit> in the bridge and surface as &mut self in
the safe API, so Rust's borrow checker statically prevents a shared
&Toolkit from being driven concurrently.
The original Send + !Sync claim was inferred from API shape. A
Verovio source audit surfaced three process-global hazards that the
binding mitigates by API design (not docs):
-
Static
Toolkit::m_humdrumBuffer— a rawchar*shared across all Toolkit instances and freed in any destructor. Mitigation: no Humdrum methods are exposed in the bridge. -
Namespace-level log globals in
src/vrv.cpp—logBuffer,logLevel,loggingToBuffer, not mutex-protected. Mitigations:-
verovio::set_log_levelis wrapped in a crate-levelMutex<()>. - Verovio's internal render paths still write to these globals on
every render. The operational answer is to silence the log:
verovio::set_log_level(LogLevel::Off)at process start.
-
-
Toolkit::SetLocale()— mutatesstd::locale::globalprocess-wide. Mitigation: not exposed in the bridge at all.
use std::sync::Arc;
use std::thread;
use verovio::{Toolkit, set_log_level, LogLevel};
set_log_level(LogLevel::Off); // silence the log races
let score: Arc<str> = Arc::from(include_str!("score.mei"));
let mut handles = Vec::new();
for page_chunk in 0..4 {
let s = Arc::clone(&score);
handles.push(thread::spawn(move || {
let mut tk = Toolkit::from_data(&s).unwrap();
// render some pages
}));
}
for h in handles {
h.join().unwrap();
}Or one worker thread fronted by a channel for low-overhead serial rendering.
Running the concurrency test suite under ThreadSanitizer
(cargo test --features verovio/sanitize-thread --test concurrency)
reports 8 distinct data races, all in Verovio's C++ with no
observable runtime impact in our tests:
| # | Where | Cause |
|---|---|---|
| 1 | vrv.cpp:193 EnableLog |
logLevel = level without sync. Mitigated by our Rust Mutex<()> (TSan can't see Rust mutexes — see below). |
| 2 | toolkit.cpp:71 |
m_humdrumBuffer = NULL on a static char*. Idempotent (same value from every thread). |
| 3, 4 | memcpy |
Unattributed, same path as #2. |
| 5 | free |
~Toolkit() racing with another constructor on the same static. |
| 6 | localtime |
Non-reentrant localtime() in Doc::GenerateMEIHeader. POSIX bug — should be localtime_r. |
| 7, 8 | doc.cpp:342-343 |
Reads from the localtime shared buffer (downstream of #6). |
The sanitize-thread feature only adds -fsanitize=thread to the
C++ compile. Rust code is not TSan-instrumented (that needs
nightly Rust with -Zsanitizer=thread). So TSan cannot see Rust's
Mutex acquire/release. A race that's actually serialized by a
Rust-side mutex still looks like a race to TSan because TSan can't
observe the happens-before edge.
That's the case for race #1 above — it's genuine in the C++ memory
model but is serialized in Rust by crate::log::set_log_level's
Mutex<()>.
To silence races 2, 5, 6, 7, 8 we'd need a global Rust Mutex around
construction and loading. We deliberately don't, because:
- No correctness impact. Idempotent writes / corrupted-date strings, no crashes, no test failures.
-
Defeats
Toolkit: Send. The whole point ofSendis that consumers can drive N renders in parallel. A global lock would serialize them. -
They're upstream bugs. The right fix is a Verovio PR
(
std::atomic<int>forlogLevel,localtime_r, non-static orthread_localm_humdrumBuffer).
A consumer needing zero TSan findings can wrap the whole Toolkit in
Arc<Mutex<Toolkit>> themselves — that's their call, not ours.
See the project-tsan-findings memory entry for the full record.