Repository navigation
Conversation
The loop guard only watches a non-consuming push followed by a non-consuming pop, and the comment next to it admitted that a non-consuming `set` can loop the same way but left it alone. A `set` is neither the push that arms the guard nor the pop that trips it, and it leaves the stack depth alone, so two contexts that `set` each other at the same byte spin in `parse_next_token` forever, with a flat stack and no allocation to show for it. Older JavaScript packages hit this on a line like `x /*`, where `expression-statement-continuation` and the "rest of the line is blanks and comments" rule both match empty and hand the position back to each other; current Sublime packages dodge it with a `branch` instead, which is why nothing in `testdata` ever caught it. The fix remembers the `(byte, context stack)` a non-consuming `set` was taken from and, on a second visit to the same state, advances one character like the push/pop guard does. The stack has to be in the key because `include:` shares contexts, so the same rule legitimately fires at one byte from many states, and keying on the rule alone costs hundreds of syntest assertions. Both syntest baselines are unchanged.
Opening a session sent a core to 100% and kept it there until maki was killed, and nothing in the transcript looked unusual. The thread named `highlight` was stuck inside a single `ParseState::parse_line` call on a 20 line JavaScript snippet, and the minimal case turned out to be `x /*`: two rules in the JavaScript syntax `bat` ships both match empty at the byte after the identifier and `set` each other's context back, forever, because syntect's loop guard only watches a push followed by a pop, and a `set` is neither. Flat memory the whole time, so it is a real cycle rather than backtracking, and the queue in `maki-highlight/src/pool.rs` is a single thread with no deadline, so that one line also starved every highlight job that came after it. Pins syntect to a `v5.3.0` backport of trishume/syntect#706, which remembers the `(byte, context stack)` a non-consuming `set` was taken from and advances a character on the second visit, exactly what the push/pop guard already does; both syntest baselines are unchanged upstream.
Opening a session sent a core to 100% and kept it there until maki was killed, and nothing in the transcript looked unusual. The thread named `highlight` was stuck inside a single `ParseState::parse_line` call on a 20 line JavaScript snippet, and the minimal case turned out to be `x /*`: two rules in the JavaScript syntax `bat` ships both match empty at the byte after the identifier and `set` each other's context back, forever, because syntect's loop guard only watches a push followed by a pop, and a `set` is neither. Flat memory the whole time, so it is a real cycle rather than backtracking, and the queue in `maki-highlight/src/pool.rs` is a single thread with no deadline, so that one line also starved every highlight job that came after it. Pins syntect to a `v5.3.0` backport of trishume/syntect#706, which remembers the `(byte, context stack)` a non-consuming `set` was taken from and advances a character on the second visit, exactly what the push/pop guard already does; both syntest baselines are unchanged upstream.
Opening a session sent a core to 100% and kept it there until maki was killed, and nothing in the transcript looked unusual. The thread named `highlight` was stuck inside a single `ParseState::parse_line` call on a 20 line JavaScript snippet, and the minimal case turned out to be `x /*`: two rules in the JavaScript syntax `bat` ships both match empty at the byte after the identifier and `set` each other's context back, forever, because syntect's loop guard only watches a push followed by a pop, and a `set` is neither. Flat memory the whole time, so it is a real cycle rather than backtracking, and the queue in `maki-highlight/src/pool.rs` is a single thread with no deadline, so that one line also starved every highlight job that came after it. Pins syntect to a `v5.3.0` backport of trishume/syntect#706, which remembers the `(byte, context stack)` a non-consuming `set` was taken from and advances a character on the second visit, exactly what the push/pop guard already does; both syntest baselines are unchanged upstream.
Opening a session sent a core to 100% and kept it there until maki was killed, and nothing in the transcript looked unusual. The thread named `highlight` was stuck inside a single `ParseState::parse_line` call on a 20 line JavaScript snippet, and the minimal case turned out to be `x /*`: two rules in the JavaScript syntax `bat` ships both match empty at the byte after the identifier and `set` each other's context back, forever, because syntect's loop guard only watches a push followed by a pop, and a `set` is neither. Flat memory the whole time, so it is a real cycle rather than backtracking, and the queue in `maki-highlight/src/pool.rs` is a single thread with no deadline, so that one line also starved every highlight job that came after it. Pins syntect to a `v5.3.0` backport of trishume/syntect#706, which remembers the `(byte, context stack)` a non-consuming `set` was taken from and advances a character on the second visit, exactly what the push/pop guard already does; both syntest baselines are unchanged upstream.
|
Independent reproducer, and a data point on the engine question.
Versions: The same two lines also hang in a separate standalone build linked with Same shape as your I have not reviewed the patch itself. |
|
mdmost now carries this patch in a vendored copy of syntect 5.3.0, with two changes to the key. You may want them here:
If you want either change, I can send a patch against this branch. |
|
bump |
|
bump |
DISCLAIMER: fully AI generated, some nice bug I found, told AI to fix quickly.
The loop guard in
parse_next_tokenonly watches a non-consuming push followed by a non-consuming pop, and the comment next to it admitted that a non-consumingsetcan loop the same way but left it alone.A
setis neither the push that arms the guard nor the pop that trips it, and it leaves the stack depth alone, so two contexts thatseteach other at the same byte spin forever. The stack stays flat and nothing is allocated, so it looks like a hang rather than a runaway.Repro
Older JavaScript packages (the ones
batships, viatwo-face) hit this on a line as small asx /*:expression-statement-continuationmatches empty because the/of/*is in its operator class, and the "rest of the line is blanks and comments then EOL" rule also matches empty, and the two hand the position back to each other. Current Sublime packages replaced thatsetpair with abranch(see sublimehq/sublime_text#5853), which is why nothing intestdatacatches it today. Anything loading older or third party syntaxes still hits it. Found in the wild by a terminal app that pinned a core forever when it restored a session containing that line.Fix
Remember the
(byte, context stack)a non-consumingsetwas taken from and, on a second visit to the same state, advance one character, the same recovery the push/pop guard already uses.The stack has to be part of the key.
include:sharesContextobjects, so the same rule legitimately fires at one byte from many states; keying on the rule alone looked simpler but cost several hundred syntest assertions across JavaScript, D and Python.advance_one_charis pulled out as a helper so both guards use the same byte-safe step instead of duplicating thechar_indices().nth(1)dance.Testing
can_parse_non_consuming_sets_that_would_loop, which hangs without the fix.cargo test --lib parsing::parser: 87 passed, existing loop tests stay green.make syntestandmake syntest-fancy: no new failures, both baselines byte for byte unchanged.cargo fmt --checkclean.