Skip to content

Fix crash when removing guitar bend hold lines - #34623

Open
ajuncosa wants to merge 1 commit into
musescore:mainfrom
ajuncosa:guitar-hold-crash
Open

Fix crash when removing guitar bend hold lines#34623
ajuncosa wants to merge 1 commit into
musescore:mainfrom
ajuncosa:guitar-hold-crash

Conversation

@ajuncosa

Copy link
Copy Markdown
Contributor

Ports: #34500

Note: it was discussed in the above 4.7 PR to also call setGenerated() on the GuitarBendHoldSegments in 5.0, but this involves changes in behaviour, so I decided to leave it alone for this fix.

Resolves: #34481

The crash:

  1. GuitarBendHold is created, owned and destroyed by its GuitarBend, not by the score. Also, GuitarBend::updateHoldLine() calls setGenerated(true) on it, but not on its segments.
  2. The hold line segment falls into Score::deleteItem's default case, which does not really remove the hold line, but a RemoveElement command is being created for it.
  3. Then, when the tie is removed, updateHoldLine() deletes the hold line and all its segments.
  4. On close, UndoStack::~UndoStack() calls cleanup(true) on the RemoveElement command, which deletes the already freed GuitarBendHoldSegment.

The fix was to add a special switch case to Score::deleteItem to avoid the step 2 above (so the hold line simply cannot be removed in itself).

@ajuncosa
ajuncosa marked this pull request as ready for review August 18, 2026 12:39
@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 29496904-db11-4236-b43e-7b341a027a49

📥 Commits

Reviewing files that changed from the base of the PR and between 929d1e9 and 3822de9.

📒 Files selected for processing (1)
  • src/engraving/editing/edit.cpp

Included review availability: Your plan includes up to 10 reviews per rolling hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Score::deleteItem now handles ElementType::GUITAR_BEND_HOLD_SEGMENT as non-removable. The element is routed to the existing branch that logs non-removable elements instead of deleting them.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the issue, cause, fix, and linked issue, but it omits the required checklist. Add the completed repository checklist, including CLA status, testing, coding guidelines, commit references, unnecessary changes, and applicable test coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The change directly prevents the crash described in issue #34481 by blocking independent removal of hold-line segments.
Out of Scope Changes check ✅ Passed The one-line change is limited to the linked issue and stated objective; no unrelated changes are present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title clearly describes the crash fix for removing guitar bend hold lines.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped musescore/muse_framework.git.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ajuncosa
ajuncosa requested a review from miiizen August 18, 2026 12:43
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.

Crash when closing score after removing guitar bend hold

1 participant