fix(display): deactivate completed process before starting a new one (GM-5)

[jniebuhr]

Summary

Controller::startProcess() assigned currentProcess without freeing an existing one. isActive() returns false once a process completes but before the next loop() tick calls deactivate() (which moves it into lastProcess). An activate() / activateGrind() in that ~100 ms window — easily hit from the AsyncTCP WebSocket handler on core 1 while completion runs on the main-loop core — overwrote and leaked the live currentProcess.

Fix: in startProcess(), if currentProcess != nullptr, call deactivate() first. This preserves the lastProcess auto-delay bookkeeping and fires the expected controller:process:end events, and never overwrites a live pointer.

Closes GM-5.

Test plan

• platformio run -e display succeeds
• platformio run -e display-headless succeeds
• On-device: repeatedly activate -> let complete -> re-activate; confirm free heap stays stable and end/start events still fire

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

• Fixed a resource management issue where active processes could be inadvertently lost in certain scenarios, improving overall system stability and reliability.

[sonarqubecloud]

Quality Gate failed

Failed conditions
D Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 769b1fe1-0fa9-4e62-a568-f8ff7b30b86e

📥 Commits

Reviewing files that changed from the base of the PR and between c2c439d and 1d91f40.

📒 Files selected for processing (1)

• src/display/core/Controller.cpp

---

📝 Walkthrough

Walkthrough

A guard is added to Controller::startProcess() to deactivate any existing non-null currentProcess before replacing it, preventing pointer orphaning when isActive() is false during concurrent multi-core access.

Changes

Process lifecycle safety

Layer / File(s)	Summary
Process deactivation guard in startProcess() src/display/core/Controller.cpp	When startProcess() encounters a non-null currentProcess but isActive() is false, deactivate() is now called to move the existing process to lastProcess and trigger end events before assigning the new process, eliminating a window where other-core activate paths could orphan the pointer.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• jniebuhr/gaggimate#714: Both PRs modify src/display/core/Controller.cpp to fix cross-core/race issues around currentProcess/lastProcess during deactivate()—main PR prevents overwriting an existing currentProcess in startProcess(), while retrieved PR prevents use-after-free in onVolumetricMeasurement() by locally capturing those pointers.
• jniebuhr/gaggimate#400: Both PRs address the same concurrency/race around currentProcess/process lifetime during brew start/stop—main PR prevents Controller::startProcess() from orphaning currentProcess, while retrieved PR adds UI-side null/bounds guards when DefaultUI::updateStatusScreen() dereferences current/last process during quick restarts.

Poem

🐰 A process once lost in the void,
Now safely deactivated before we employ,
Guard checks the current, then hands it off clean,
No orphaned pointers tween cores unseen! 🧵

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: preventing process loss by deactivating a completed process before starting a new one. It directly matches the changeset's primary objective.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/startprocess-process-leak

---

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 and usage tips.}