Skip to content

fix: prevent re-entrant reset crash in GCodeViewer::load() - #611

Open
zackaree-shen wants to merge 1 commit into
mainfrom
bugfix_repeated_gcode_import_crash
Open

fix: prevent re-entrant reset crash in GCodeViewer::load()#611
zackaree-shen wants to merge 1 commit into
mainfrom
bugfix_repeated_gcode_import_crash

Conversation

@zackaree-shen

Copy link
Copy Markdown

Description

Fixes a crash that occurs when importing/loading G-code repeatedly. The modal progress dialog in load_toolpaths() pumps the wxWidgets event loop, which allows dispatched events (e.g. a plate switch or re-import) to re-enter GCodeViewer::reset() via reset_gcode_toolpaths() mid-flight. This zeroes m_moves_count, causing an unsigned underflow in reserve(m_moves_count - biased_seams_ids.size()) which throws std::length_error.

Root Cause

load_toolpaths() displays a ProgressDialog and calls Update() periodically, pumping the event loop. A re-entrant reset() call zeroes m_moves_count while load_toolpaths() is still running, and the subsequent reserve() / loop bound computation wraps around.

Changes

  • m_loading guard on reset(): Added an m_loading flag armed via RAII ScopeGuard around the load_toolpaths() call. reset() early-returns when m_loading is set, preventing re-entrant zeroing. The initial reset() at the top of load() runs before the flag is armed so cleanup is not skipped.
  • ScopeGuard for gcode_result.unlock(): Replaced manual unlock() calls in load() and refresh() with ScopeGuard for exception safety on all exit paths.
  • Defensive clamp on ssid_count: Added a bounds check in load_toolpaths() so the reserve() and loop use a clamped ssid_count instead of a raw subtraction, as a belt-and-suspenders measure.

Impact

GCodeViewer.cpp (27 lines), GCodeViewer.hpp (1 line). No new dependencies, no API changes.

Screenshots/Recordings/Graphs

N/A -- crash fix, no UI changes.

Tests

  • Manual: Repeatedly import the same G-code file (5+ times) -- no crash
  • Manual: Switch between Preview and Prepare rapidly during/after slicing -- no crash
  • Manual: Cancel slicing mid-process then re-preview -- no crash
  • CI build passes (pending)

The modal progress dialog in load_toolpaths() pumps the event loop,
allowing dispatched events to re-enter reset() and zero m_moves_count.
This causes unsigned underflow in reserve(m_moves_count -
biased_seams_ids.size()), throwing std::length_error.

Guard reset() with an m_loading flag armed via RAII ScopeGuard,
and move reset() before the flag is set. Replace manual
gcode_result.unlock() with ScopeGuard for exception safety.
Also add a defensive clamp on the ssid_count computation in
load_toolpaths() as a belt-and-suspenders measure.
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.

2 participants