You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Split out of #1316. TokenizerHostObject guards its tokenizer with an exclusive std::mutex even though every op (encode, decode, id_to_piece, piece_to_id, vocab_size) is const upstream. Ideally it would use a std::shared_mutex so pure reads run concurrently (like TensorHostObject), with only dispose taking a unique lock.
That change is not safe against the currently vendored tokenizers library. const does not imply thread-safe here:
HF tokenizers encode via the PCRE2 fallback — RE2 rejects the lookahead in HF pre-tokenizer regexes, so create_regex falls back to Pcre2Regex.
The vendored Pcre2Regex holds a single pcre2_match_data* match_data_member that pcre2_match() writes into during the constfind_all(). Two concurrent encode() calls on one tokenizer would race on that shared buffer (silent result corruption / heap overflow inside PCRE2).
The upstream fix
pytorch-labs/tokenizers fixed exactly this in commit c321bca3d ("Fix Pcre2Regex thread-safety bug + regression test", 2026-05-05) by allocating match_dataper call and dropping the member from the header.
Task
Bump the vendored tokenizers library to a revision that includes c321bca3d.
Switch TokenizerHostObject::mutex_ from std::mutex to std::shared_mutex, taking a shared lock on the read ops and a unique lock only on dispose. The generic core::tryLock<> helper (added in [RNE Rewrite] Clean up the tokenizer host object: locking, error unwrapping and native default arg #1316) already supports both lock kinds, so this is a one-line member change plus swapping std::unique_lock → std::shared_lock at the read sites.
Remove the TODO comment referencing this issue in cpp/extensions/nlp/tokenizer.h.
Until then the exclusive std::mutex is intentional and must stay.
Background
Split out of #1316.
TokenizerHostObjectguards its tokenizer with an exclusivestd::mutexeven though every op (encode,decode,id_to_piece,piece_to_id,vocab_size) isconstupstream. Ideally it would use astd::shared_mutexso pure reads run concurrently (likeTensorHostObject), with onlydisposetaking a unique lock.That change is not safe against the currently vendored
tokenizerslibrary.constdoes not imply thread-safe here:create_regexfalls back toPcre2Regex.Pcre2Regexholds a singlepcre2_match_data* match_data_member thatpcre2_match()writes into during theconstfind_all(). Two concurrentencode()calls on one tokenizer would race on that shared buffer (silent result corruption / heap overflow inside PCRE2).The upstream fix
pytorch-labs/tokenizersfixed exactly this in commitc321bca3d("Fix Pcre2Regex thread-safety bug + regression test", 2026-05-05) by allocatingmatch_dataper call and dropping the member from the header.Task
tokenizerslibrary to a revision that includesc321bca3d.TokenizerHostObject::mutex_fromstd::mutextostd::shared_mutex, taking a shared lock on the read ops and a unique lock only ondispose. The genericcore::tryLock<>helper (added in [RNE Rewrite] Clean up the tokenizer host object: locking, error unwrapping and native default arg #1316) already supports both lock kinds, so this is a one-line member change plus swappingstd::unique_lock→std::shared_lockat the read sites.TODOcomment referencing this issue incpp/extensions/nlp/tokenizer.h.Until then the exclusive
std::mutexis intentional and must stay.