Repository navigation
VPAAMP-1495 uveExecuter quit unexpectedly - #2130
Merged
Merged
Conversation
pstroffolino
force-pushed
the
feature/VPAAMP-1495
branch
from
October 23, 2026 22:39
934e1ed to
b8851db
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Shutdown handling can leave a worker accessing destroyed player state and does not reliably prevent termination.
2 open issues
What changed in this PR
This PR aims to prevent the macOS crash popup when uveExecutor finishes by changing pause-position monitor shutdown in AAMP.
Changes:
- Catch exceptions from monitor shutdown during player destruction.
- Log mutex errors and detach the monitor thread when they occur.
| File | Description |
|---|---|
priv_aamp.cpp |
Adds exception handling and a thread-detachment fallback during monitor shutdown. |
Review effort: Balanced
StopPausePositionMonitoring() acquires a mutex via std::unique_lock.
During process exit, _aamp_term() calls Stop() then deletes the player.
In a race between AAMP internal threads and _aamp_term(), the mutex can
be in an invalid state when the destructor calls
StopPausePositionMonitoring() a second time, causing std::unique_lock to
throw std::system_error ("mutex lock failed: Invalid argument").
Destructors are implicitly noexcept, so an uncaught exception propagates
to std::terminate() and aborts the process, even though the session has
already finished successfully.
Fix 1 - StopPausePositionMonitoring (primary fix):
Wrap the std::unique_lock construction in try/catch(std::system_error).
When the mutex is invalid, detach the thread instead of blocking on
join(); the thread exits imminently with the process.
Fix 2 - PrivateInstanceAAMP destructor (defence-in-depth):
Wrap the StopPausePositionMonitoring call in try/catch(...) so no
exception can escape the destructor body and reach std::terminate().
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
pstroffolino
force-pushed
the
feature/VPAAMP-1495
branch
from
October 23, 2026 22:43
b8851db to
93b6bcb
Compare
…r, flag teardown exceptions Reason for change: uveExecuter intermittently aborts (SIGABRT via std::terminate) in ~PrivateInstanceAAMP during process teardown. The previous patch caught the exception in the destructor and detached the monitor thread on mutex failure; review noted that a still-joinable std::thread member terminates anyway and a detached worker keeps using the destroyed object. Changes: - StopPausePositionMonitoring never detaches; it cancels under the monitor mutex (notify_all) and always joins the worker. - New mPausePositionLifecycleMutex serializes Start/StopPausePositionMonitoring so a monitor cannot be restarted while another thread stops/joins it (StopPausePositionMonitoringLocked used by Start). - Destructor keeps a last-resort try/catch that calls AampTeardownExceptionHook, which prints an "AAMP_TEARDOWN_EXCEPTION:" marker to stderr (breakpoint-able) so occurrences are visible to test harnesses. Known open issue: an abort is still seen intermittently (crash report 2026-10-26 13:12). Stack: __cxa_finalize_ranges -> _aamp_term -> ~PlayerInstanceAAMP -> ~PrivateInstanceAAMP -> std::terminate, i.e. the player is destroyed during static destruction at process exit; investigation ongoing. Test Procedure: aamp_test_internal tools/ppjstestrunner run_ppjs_test_runner.sh (basic, switching, stitching, seek), repeated; check for AAMP_TEARDOWN_EXCEPTION and ~/Library/Logs/DiagnosticReports/uveExecuter-*.ips. Risks: Low-Medium - teardown/threading change in PauseAt monitor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
_aamp_term() is __attribute__((destructor(101))): priority 101 is the lowest, so it fires after all C++ static-storage-duration objects are destroyed. When it called `delete _allocated_aamp` at that point, ~PrivateInstanceAAMP() tried to lock `gMutex` (static std::mutex, already destroyed) -- std::system_error escaped the noexcept destructor -- std::terminate() -- SIGABRT. Fix 1 (UVEBinding.cpp / uveExecuter): call aamp_UnloadJSController(ctx) before every JSGlobalContextRelease() path. aamp_UnloadJSController -> aamp_UnloadJS -> AAMP_finalize stops and deletes _allocated_aamp (sets it NULL) while all statics are still alive. _aamp_term then finds NULL and returns immediately. Fix 2 (jsbindings.cpp): move ClearAAMPPlayerInstances() inside the `if (_allocated_aamp != NULL)` guard in _aamp_term. AAMPMediaPlayer_JS::_jsMediaPlayerInstances is a static that is also destroyed before destructor(101) runs; calling size() on it then is UB. After the explicit teardown from Fix 1, _aamp_term is a pure no-op so the call is skipped entirely. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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.
VPAAMP-1495 uveExecuter quit unexpectedly
Reason for Change: avoid error popup when uveExecutor completes