iOS: notify the ADM observer when an enable operation rolls back after will-enable - #270
Open
hiroshihorie wants to merge 1 commit into
Open
iOS: notify the ADM observer when an enable operation rolls back after will-enable#270hiroshihorie wants to merge 1 commit into
hiroshihorie wants to merge 1 commit into
Conversation
…able OnEngineWillEnable is invoked before the fallible parts of an enable operation: permission and category checks, node configuration, and engine start. When any of those failed, the rollback replayed only the constructive engine actions and never told the observer, so an observer that configured and activated the audio session for the enable was left holding an activation for an engine state that never materialized. It leaked until the next retry or stop. Push a rollback action right after a successful will-enable that fires OnEngineDidDisable with the previous engine state. Existing observers already implement did-disable as the release edge, so they get correct compensation without adopting a new callback. The result is ignored since the rollback itself cannot be aborted. Applies to both the device and manual rendering paths.
There was a problem hiding this comment.
Pull request overview
This PR fixes an iOS observer lifecycle gap where OnEngineWillEnable can succeed but a later enable step fails, causing the engine to roll back without notifying the ADM observer—potentially leaking audio session activation in observers that acquire resources in willEnable.
Changes:
- Add a rollback action immediately after successful
OnEngineWillEnablethat notifies the observer viaOnEngineDidDisablewith the previous engine state when a later enable step fails. - Apply the same rollback-notification behavior to both manual rendering and device rendering paths.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1855
to
+1859
| if (observer_ != nullptr) { | ||
| LOGW() << "Enable rolled back after OnEngineWillEnable, notifying observer (Manual)"; | ||
| observer_->OnEngineDidDisable(engine_manual_input_, state.prev.IsOutputEnabled(), | ||
| state.prev.IsInputEnabled()); | ||
| } |
Comment on lines
+2297
to
+2301
| if (observer_ != nullptr) { | ||
| LOGW() << "Enable rolled back after OnEngineWillEnable, notifying observer"; | ||
| observer_->OnEngineDidDisable(engine_device_, state.prev.IsOutputEnabled(), | ||
| state.prev.IsInputEnabled()); | ||
| } |
cloudwebrtc
approved these changes
Jul 16, 2026
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
OnEngineWillEnableruns before permission and category checks, node configuration, and engine startup. If a later step fails, rollback never notifies the observer. Observers that configure or activate the audio session inwillEnablecan therefore retain resources for an engine state that was never applied.Change
After a successful
OnEngineWillEnable, register a rollback action that callsOnEngineDidDisablewith the previous state. This covers both device and manual rendering paths. The result is ignored because rollback cannot be cancelled.OnEngineDidDisablemay now represent an attempted enable returning to the previous state, not only a completed disable.Testing
Built
framework_objcfor an arm64 iOS device. Reproduced by enabling input with an incompatible playback-only category and verified that rollback now notifies the observer.