From 93b6bcb77e07424701fe5ffe9f0aa5bd3859fa09 Mon Sep 17 00:00:00 2001 From: Philip Stroffolino Date: Fri, 23 Oct 2026 15:35:11 -0400 Subject: [PATCH 1/3] Fix uveExecuter crash (abort) during process teardown 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> --- priv_aamp.cpp | 48 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/priv_aamp.cpp b/priv_aamp.cpp index 776dc9bd0e..9b8ebd8a00 100644 --- a/priv_aamp.cpp +++ b/priv_aamp.cpp @@ -1993,7 +1993,19 @@ PrivateInstanceAAMP::PrivateInstanceAAMP(AampConfig *config) : mReportProgressPo PrivateInstanceAAMP::~PrivateInstanceAAMP() { mAampTrackWorkerManager.reset(); - StopPausePositionMonitoring("AAMP destroyed"); + // StopPausePositionMonitoring() can throw std::system_error when its internal + // mutex is in an invalid state during process teardown (see the function body for + // details). Destructors are implicitly noexcept, so an uncaught exception would + // call std::terminate() and abort the process. The try-catch here is + // defence-in-depth: the primary fix lives inside StopPausePositionMonitoring itself. + try + { + StopPausePositionMonitoring("AAMP destroyed"); + } + catch (...) + { + AAMPLOG_WARN("StopPausePositionMonitoring threw during AAMP destruction (ignored)"); + } PlayerCCManager::GetInstance()->Release(mCCId); mCCId = 0; { @@ -2294,17 +2306,33 @@ void PrivateInstanceAAMP::StopPausePositionMonitoring(std::string reason) { if (mPausePositionMonitoringThreadID.joinable()) { - std::unique_lock lock(mPausePositionMonitorMutex); - - if (mPausePositionMilliseconds != AAMP_PAUSE_POSITION_INVALID_POSITION) + try { - long long positionMs = GetPositionMilliseconds(); - AAMPLOG_WARN("PLAYER[%d] Stop position monitoring, reason: '%s', current position %lldms, requested position %lldms, rate %f", - mPlayerId, reason.c_str(), positionMs, mPausePositionMilliseconds.load(), rate); - mPausePositionMilliseconds = AAMP_PAUSE_POSITION_INVALID_POSITION; - mPausePositionMonitorCV.notify_one(); + std::unique_lock lock(mPausePositionMonitorMutex); + + if (mPausePositionMilliseconds != AAMP_PAUSE_POSITION_INVALID_POSITION) + { + long long positionMs = GetPositionMilliseconds(); + AAMPLOG_WARN("PLAYER[%d] Stop position monitoring, reason: '%s', current position %lldms, requested position %lldms, rate %f", + mPlayerId, reason.c_str(), positionMs, mPausePositionMilliseconds.load(), rate); + mPausePositionMilliseconds = AAMP_PAUSE_POSITION_INVALID_POSITION; + mPausePositionMonitorCV.notify_one(); + } + lock.unlock(); + } + catch (const std::system_error& e) + { + // Mutex lock failed (e.g. "Invalid argument") during process teardown. This + // happens when the monitoring thread races against _aamp_term(): Stop() already + // joined the thread on one path, but a new monitoring cycle started in between, + // leaving the thread joinable again with internal state partially torn down. + // We cannot signal the CV, so detach instead of blocking on join(). The thread + // will exit shortly as the process winds down. + AAMPLOG_WARN("PLAYER[%d] StopPausePositionMonitoring mutex error during teardown (%s) — detaching thread", + mPlayerId, e.what()); + mPausePositionMonitoringThreadID.detach(); + return; } - lock.unlock(); mPausePositionMonitoringThreadID.join(); AAMPLOG_TRACE("joined PositionMonitor"); } From 7067f785d3eca3dcc6a3b2b0a144b3e2716138d2 Mon Sep 17 00:00:00 2001 From: Philip Stroffolino Date: Mon, 26 Oct 2026 13:16:09 -0400 Subject: [PATCH 2/3] VPAAMP-1495 uveExecuter quit unexpectedly: join pause-position monitor, 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> --- priv_aamp.cpp | 64 +++++++++++++++++++++++++++++++-------------------- priv_aamp.h | 2 ++ 2 files changed, 41 insertions(+), 25 deletions(-) diff --git a/priv_aamp.cpp b/priv_aamp.cpp index 9b8ebd8a00..ef134422a5 100644 --- a/priv_aamp.cpp +++ b/priv_aamp.cpp @@ -1990,21 +1990,36 @@ PrivateInstanceAAMP::PrivateInstanceAAMP(AampConfig *config) : mReportProgressPo /** * @brief PrivateInstanceAAMP Destructor */ +/** + * @brief Reports an exception escaping teardown of the pause position monitor. + * Has external linkage and a stable name so a debugger breakpoint can be set on it; the marker + * line is written straight to stderr so test harnesses see it regardless of AAMP log level. + */ +void AampTeardownExceptionHook(int playerId, const char *what, bool stillJoinable) +{ + fprintf(stderr, "AAMP_TEARDOWN_EXCEPTION: PLAYER[%d] StopPausePositionMonitoring threw during destruction: %s%s\n", + playerId, what, stillJoinable ? " (monitor thread still joinable, std::terminate will follow)" : ""); + fflush(stderr); + AAMPLOG_ERR("AAMP_TEARDOWN_EXCEPTION: PLAYER[%d] %s", playerId, what); +} + PrivateInstanceAAMP::~PrivateInstanceAAMP() { mAampTrackWorkerManager.reset(); - // StopPausePositionMonitoring() can throw std::system_error when its internal - // mutex is in an invalid state during process teardown (see the function body for - // details). Destructors are implicitly noexcept, so an uncaught exception would - // call std::terminate() and abort the process. The try-catch here is - // defence-in-depth: the primary fix lives inside StopPausePositionMonitoring itself. + // Last resort: StopPausePositionMonitoring() should not throw, but an intermittent + // teardown abort was still observed. Report it loudly instead of letting it escape this noexcept + // destructor. If the thread is still joinable, its member destructor will terminate anyway. try { StopPausePositionMonitoring("AAMP destroyed"); } + catch (const std::exception &e) + { + AampTeardownExceptionHook(mPlayerId, e.what(), mPausePositionMonitoringThreadID.joinable()); + } catch (...) { - AAMPLOG_WARN("StopPausePositionMonitoring threw during AAMP destruction (ignored)"); + AampTeardownExceptionHook(mPlayerId, "unknown exception", mPausePositionMonitoringThreadID.joinable()); } PlayerCCManager::GetInstance()->Release(mCCId); mCCId = 0; @@ -2267,7 +2282,10 @@ void PrivateInstanceAAMP::RunPausePositionMonitoring(void) */ void PrivateInstanceAAMP::StartPausePositionMonitoring(long long pausePositionMilliseconds) { - StopPausePositionMonitoring("Start new pos monitor"); + // Serialize with StopPausePositionMonitoring() so a monitor cannot be (re)started while another + // thread is stopping/joining it, which would leave a joinable thread behind at teardown. + std::lock_guard lifecycleLock(mPausePositionLifecycleMutex); + StopPausePositionMonitoringLocked("Start new pos monitor"); if (pausePositionMilliseconds < 0) { @@ -2303,36 +2321,32 @@ void PrivateInstanceAAMP::StartPausePositionMonitoring(long long pausePositionMi * @brief stop the PausePositionMonitoring thread used for PauseAt functionality */ void PrivateInstanceAAMP::StopPausePositionMonitoring(std::string reason) +{ + std::lock_guard lifecycleLock(mPausePositionLifecycleMutex); + StopPausePositionMonitoringLocked(reason); +} + +/** + * @brief cancel and always join the PausePositionMonitoring thread; caller holds mPausePositionLifecycleMutex + */ +void PrivateInstanceAAMP::StopPausePositionMonitoringLocked(const std::string &reason) { if (mPausePositionMonitoringThreadID.joinable()) { - try { - std::unique_lock lock(mPausePositionMonitorMutex); - + // Cancel under the monitor mutex so the worker cannot miss the wake-up between its + // predicate check and wait_until(). The mutex is a member and valid for the object's lifetime. + std::lock_guard lock(mPausePositionMonitorMutex); if (mPausePositionMilliseconds != AAMP_PAUSE_POSITION_INVALID_POSITION) { long long positionMs = GetPositionMilliseconds(); AAMPLOG_WARN("PLAYER[%d] Stop position monitoring, reason: '%s', current position %lldms, requested position %lldms, rate %f", mPlayerId, reason.c_str(), positionMs, mPausePositionMilliseconds.load(), rate); mPausePositionMilliseconds = AAMP_PAUSE_POSITION_INVALID_POSITION; - mPausePositionMonitorCV.notify_one(); } - lock.unlock(); - } - catch (const std::system_error& e) - { - // Mutex lock failed (e.g. "Invalid argument") during process teardown. This - // happens when the monitoring thread races against _aamp_term(): Stop() already - // joined the thread on one path, but a new monitoring cycle started in between, - // leaving the thread joinable again with internal state partially torn down. - // We cannot signal the CV, so detach instead of blocking on join(). The thread - // will exit shortly as the process winds down. - AAMPLOG_WARN("PLAYER[%d] StopPausePositionMonitoring mutex error during teardown (%s) — detaching thread", - mPlayerId, e.what()); - mPausePositionMonitoringThreadID.detach(); - return; + mPausePositionMonitorCV.notify_all(); } + // Never detach: the worker uses this object, so it must be joined before the owner can be destroyed. mPausePositionMonitoringThreadID.join(); AAMPLOG_TRACE("joined PositionMonitor"); } diff --git a/priv_aamp.h b/priv_aamp.h index 1d85a2034c..ffc99550e8 100644 --- a/priv_aamp.h +++ b/priv_aamp.h @@ -3858,6 +3858,7 @@ class PrivateInstanceAAMP : public DrmCallbacks, public std::enable_shared_from_ * @param[in] reason - Reason why the pause position monitoring is stopped */ void StopPausePositionMonitoring(std::string reason); + void StopPausePositionMonitoringLocked(const std::string &reason); /** * @fn WaitForDiscontinuityProcessToComplete @@ -4467,6 +4468,7 @@ class PrivateInstanceAAMP : public DrmCallbacks, public std::enable_shared_from_ */ void StopLatencyMonitor(); + std::mutex mPausePositionLifecycleMutex; // Serializes start/stop/join of the PausePositionMonitoring thread std::mutex mPausePositionMonitorMutex; // Mutex lock for PausePosition condition variable std::condition_variable mPausePositionMonitorCV; // Condition Variable to signal to stop PausePosition monitoring std::thread mPausePositionMonitoringThreadID; // Thread Id of the PausePositionMonitoring thread From 32bc0214a21037c08063744e121e300026b4ac45 Mon Sep 17 00:00:00 2001 From: Philip Stroffolino Date: Mon, 26 Oct 2026 13:41:09 -0400 Subject: [PATCH 3/3] VPAAMP-1495 Fix SIGABRT: tear down AAMP player before static destruction _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> --- jsbindings/jsbindings.cpp | 9 +++++++-- test/jsBindingTest/UVEBinding.cpp | 8 ++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/jsbindings/jsbindings.cpp b/jsbindings/jsbindings.cpp index 62a6c07120..d841708d2c 100644 --- a/jsbindings/jsbindings.cpp +++ b/jsbindings/jsbindings.cpp @@ -4926,8 +4926,13 @@ void __attribute__ ((destructor(101))) _aamp_term() LOG_WARN_EX("stopped aamp"); delete _allocated_aamp; _allocated_aamp = NULL; + //Clear any active js mediaplayer instances on term. + // Guarded here: when aamp_UnloadJSController() has already been called from + // main() (the expected path), _allocated_aamp is NULL and we skip this call + // entirely. That matters because destructor(101) fires after C++ static + // object destructors, so AAMPMediaPlayer_JS::_jsMediaPlayerInstances may + // already be destroyed — accessing it then would be undefined behaviour. + ClearAAMPPlayerInstances(); } - //Clear any active js mediaplayer instances on term - ClearAAMPPlayerInstances(); } #endif diff --git a/test/jsBindingTest/UVEBinding.cpp b/test/jsBindingTest/UVEBinding.cpp index 7f080c1b2d..27f0d36c3b 100644 --- a/test/jsBindingTest/UVEBinding.cpp +++ b/test/jsBindingTest/UVEBinding.cpp @@ -38,6 +38,7 @@ // AAMP JS registration extern "C" void aamp_LoadJSController(JSGlobalContextRef context); +extern "C" void aamp_UnloadJSController(JSGlobalContextRef context); static GMainLoop* gMainLoop = nullptr; @@ -924,6 +925,7 @@ static int main_func(int argc, char** argv) if (!evaluateGlobalScript(ctx, preloads[i - 1], argv[i])) { interval_shutdown(); http_shutdown(); + aamp_UnloadJSController(ctx); JSGlobalContextRelease(ctx); return 1; } @@ -954,6 +956,12 @@ static int main_func(int argc, char** argv) g_main_loop_unref(gMainLoop); interval_shutdown(); http_shutdown(); + // Explicitly tear down the AAMP player while C++ statics (gMutex, gActivePrivAAMPs, + // PlayerCCManager, AampStreamSinkManager, ...) are still alive. _aamp_term() is a + // __attribute__((destructor(101))) that fires very late — after those statics are + // destroyed — so it must find _allocated_aamp already NULL or it will crash trying + // to lock a destroyed mutex (SIGABRT / std::terminate). + aamp_UnloadJSController(ctx); JSGlobalContextRelease(ctx); return 0;