Print planned blocks from the grbl thread, not the simulator thread - #26
Merged
Merged
Conversation
printBlock() reads grblHAL's planner through plan_get_recent_block() (block_buffer.head->prev), but it was called from grbl_per_byte() on the simulator thread - unconditionally whenever no client is connected, which includes startup. plan_reset() on the grbl thread malloc()s and relinks that ring, so the simulator thread could read `head` before `prev` was linked and dereference uninitialized heap: SIGSEGV in printBlock(). WSL crash dumps from real runs all showed sim_loop -> printBlock -> plan_get_recent_block. Launching the simulator 400 times segfaulted 11 of them at startup before this change, 0 of 400 after. The call moves to sim_process_realtime(), the grbl.on_execute_realtime hook that already runs on the grbl thread each main-loop pass, so the planner is only read by the thread that mutates it. The existing rule of not printing blocks to a live console in socket mode is kept. Block output verified unchanged in both socket (-b file) and stdin (stdout) mode. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Problem
The simulator occasionally segfaults, most often just after startup. Every crash dump I captured (7 from real runs, 5 of them from a plain
masterbuild) has the same stack on the simulator thread:printBlock()reads the planner throughplan_get_recent_block()(block_buffer.head->prev), but it's called fromgrbl_per_byte()on the simulator thread. That call is unconditional whenever no client is connected, which includes startup. Meanwhileplan_reset()on the grbl threadmalloc()s the block ring and relinks it. The simulator thread can therefore readheadbeforeprevis linked, get uninitialized heap as a non-NULL block pointer, and crash dereferencing it.Fix
The
printBlock()call moves intosim_process_realtime(), thegrbl.on_execute_realtimehook that already runs on the grbl thread on every main-loop pass. The planner is then only read by the thread that modifies it. The existing rule of not printing blocks to a live console in socket mode is unchanged.Evidence
400 launches of
grblHAL_sim -p … -n -t 0 -b <file>, counting segfaults in the first 250 ms:master91eda77master+ this commitprintBlock()code plus #25-b filelogs one line per move with the same step counts; stdin mode prints blocks to stdout and exits cleanly on^F.-pon every push, this crash was the cause of intermittent CI failures, and it hasn't recurred with this change.Notes
masterand touches different code.settings_write_build_info()in core'ssettings.c. It copiessizeof(stored_line_t)bytes fromBUILD_INFO, a string literal that can be a single byte, an over-read on first boot.🤖 Generated with Claude Code