Fix use-after-free in UndoableTransaction::unwind

[ajuncosa]

Resolves: #34717

The issue reported a crash when clicking "cancel" from the instrument change dialog inside a part. This happened because:

• EngravingItem::linkedClone() adds a Link command before the linked clone's AddElement
• Then, UndoableTransaction::unwind() rolled back the cancelled transaction one command at a time, undoing and freeing each.
• unwind freed the AddElement and its items, and the ~EngravingObject destructor freed the LinkedObjects list pointer once it was empty.
• Lastly, by the time Link::undo() was called, it called unlink() on the freed object and on a LinkedObjects list its destructor had already deleted.

unwind() now undoes every command first and only frees them once all the undos are done.

[coderabbitai]

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: 6ddc0ca7-8c29-4e78-875a-2b4dcb35617e

📥 Commits

Reviewing files that changed from the base of the PR and between f8e0c3d and 7ac5b2b.

📒 Files selected for processing (1)

• src/engraving/editing/transaction/undostack.cpp

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

---

📝 Walkthrough

Walkthrough

UndoableTransaction::unwind now undoes all commands before it performs command cleanup and deletion. The implementation stores the commands in a temporary vector during undo processing.

Merge Risk: ⚪ Minimal · up to 7ac5b

The rollback path now keeps commands alive until all undo operations finish, preventing the cancellation crash caused by premature cleanup. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains issue #34717 and the crash mechanism, but all required checklist items remain unchecked.	Complete the repository checklist accurately. Check applicable items, provide the CLA username, and document testing, commit references, prior attempts, and unit or visual test coverage where applicable.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: fixing a use-after-free in UndoableTransaction::unwind.
Linked Issues check	✅ Passed	The change directly addresses issue #34717 by preventing Link::undo() from accessing objects and LinkedObjects lists freed by earlier command cleanup.
Out of Scope Changes check	✅ Passed	The change is limited to UndoableTransaction::unwind and is directly related to the linked crash. No unrelated scope is evident.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Warning

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

• X
• Mastodon
• Reddit
• LinkedIn

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