From f69971179ca1b9a119ed76ad95d6fdbb89d2e0bc Mon Sep 17 00:00:00 2001 From: kealdish Date: Thu, 5 Mar 2020 19:40:44 +0800 Subject: [PATCH] Fix potential crash due to race condition --- .../Handler/WCPowerConsumeStackCollector.mm | 6 +++-- .../Monitors/KSCrashMonitor_CPPException.cpp | 9 ++++--- .../Monitors/KSCrashMonitor_Deadlock.m | 6 +++-- .../Monitors/KSCrashMonitor_MachException.c | 6 +++-- .../Monitors/KSCrashMonitor_NSException.m | 6 +++-- .../Monitors/KSCrashMonitor_Signal.c | 6 +++-- .../Recording/Monitors/KSCrashMonitor_User.c | 12 ++++++--- .../Recording/Tools/KSMachineContext.c | 27 ++++++++----------- .../Recording/Tools/KSMachineContext.h | 5 ++-- 9 files changed, 48 insertions(+), 35 deletions(-) diff --git a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/CrashBlockPlugin/Main/BlockMonitor/Handler/WCPowerConsumeStackCollector.mm b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/CrashBlockPlugin/Main/BlockMonitor/Handler/WCPowerConsumeStackCollector.mm index dcb82c86e..59359b16e 100644 --- a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/CrashBlockPlugin/Main/BlockMonitor/Handler/WCPowerConsumeStackCollector.mm +++ b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/CrashBlockPlugin/Main/BlockMonitor/Handler/WCPowerConsumeStackCollector.mm @@ -761,7 +761,9 @@ - (StackInfo)getStackInfoWithThreadCount:(mach_msg_type_number_t)cost_cpu_thread } } - ksmc_suspendEnvironment(); + thread_act_array_t threads = NULL; + mach_msg_type_number_t numThreads = 0; + ksmc_suspendEnvironment(&threads, &numThreads); for (int i = 0; i < cost_cpu_thread_count; i++) { thread_t current_thread = cost_cpu_thread_list[i]; uintptr_t backtrace_buffer[maxEntries]; @@ -780,7 +782,7 @@ - (StackInfo)getStackInfoWithThreadCount:(mach_msg_type_number_t)cost_cpu_thread } } } - ksmc_resumeEnvironment(); + ksmc_resumeEnvironment(threads, numThreads); for (int i = 0; i < cost_cpu_thread_count; i++) { if (async_stack_trace_array[i] != NULL && async_stack_trace_array_count[i] != 0) { diff --git a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_CPPException.cpp b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_CPPException.cpp index 65e2ceadc..69a20c0ea 100644 --- a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_CPPException.cpp +++ b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_CPPException.cpp @@ -116,7 +116,10 @@ void my_cxa_throw(void* thrown_exception, std::type_info* tinfo, void (*dest)(vo static void CPPExceptionTerminate(void) { - ksmc_suspendEnvironment(); + thread_act_array_t threads = NULL; + mach_msg_type_number_t numThreads = 0; + ksmc_suspendEnvironment(&threads, &numThreads); + KSLOG_DEBUG("Trapped c++ exception"); const char* name = NULL; std::type_info* tinfo = __cxxabiv1::__cxa_current_exception_type(); @@ -190,14 +193,14 @@ catch(TYPE value)\ ksTmpSingal.si_pid = (int)ksmc_getThreadFromContext(machineContext); kscm_handleException(crashContext); - ksmc_resumeEnvironment(); + ksmc_resumeEnvironment(threads, numThreads); kscm_innerHandleSignal(&ksTmpSingal); kscm_handleSignal(&ksTmpSingal); } else { KSLOG_DEBUG("Detected NSException. Letting the current NSException handler deal with it."); - ksmc_resumeEnvironment(); + ksmc_resumeEnvironment(threads, numThreads); } diff --git a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_Deadlock.m b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_Deadlock.m index d6fd954b9..1102b970a 100644 --- a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_Deadlock.m +++ b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_Deadlock.m @@ -107,7 +107,9 @@ - (void) watchdogAnswer - (void) handleDeadlock { - ksmc_suspendEnvironment(); + thread_act_array_t threads = NULL; + mach_msg_type_number_t numThreads = 0; + ksmc_suspendEnvironment(&threads, &numThreads); kscm_notifyFatalExceptionCaptured(false); KSMC_NEW_CONTEXT(machineContext); @@ -127,7 +129,7 @@ - (void) handleDeadlock crashContext->stackCursor = &stackCursor; kscm_handleException(crashContext); - ksmc_resumeEnvironment(); + ksmc_resumeEnvironment(threads, numThreads); KSLOG_DEBUG(@"Calling abort()"); abort(); diff --git a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_MachException.c b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_MachException.c index 9ce76d569..6202b1a14 100644 --- a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_MachException.c +++ b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_MachException.c @@ -298,7 +298,9 @@ static void* handleExceptions(void* const userData) exceptionMessage.code[0], exceptionMessage.code[1]); if(g_isEnabled) { - ksmc_suspendEnvironment(); + thread_act_array_t threads = NULL; + mach_msg_type_number_t numThreads = 0; + ksmc_suspendEnvironment(&threads, &numThreads); g_isHandlingCrash = true; kscm_notifyFatalExceptionCaptured(true); @@ -367,7 +369,7 @@ static void* handleExceptions(void* const userData) KSLOG_DEBUG("Crash handling complete. Restoring original handlers."); g_isHandlingCrash = false; - ksmc_resumeEnvironment(); + ksmc_resumeEnvironment(threads, numThreads); kscm_innerHandleSignal(&ksTmpSingal); kscm_handleSignal(&ksTmpSingal); } diff --git a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_NSException.m b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_NSException.m index 8a0120717..439390bbc 100644 --- a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_NSException.m +++ b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_NSException.m @@ -62,7 +62,9 @@ static void handleException(NSException* exception) KSLOG_DEBUG(@"Trapped exception %@", exception); if(g_isEnabled) { - ksmc_suspendEnvironment(); + thread_act_array_t threads = NULL; + mach_msg_type_number_t numThreads = 0; + ksmc_suspendEnvironment(&threads, &numThreads); kscm_notifyFatalExceptionCaptured(false); KSLOG_DEBUG(@"Filling out context."); @@ -97,7 +99,7 @@ static void handleException(NSException* exception) KSLOG_DEBUG(@"Calling main crash handler."); kscm_handleException(crashContext); - ksmc_resumeEnvironment(); + ksmc_resumeEnvironment(threads, numThreads); kscm_innerHandleSignal(&ksTmpSingal); kscm_handleSignal(&ksTmpSingal); diff --git a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_Signal.c b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_Signal.c index e56380af6..98e84599a 100644 --- a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_Signal.c +++ b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_Signal.c @@ -84,7 +84,9 @@ static void handleSignal(int sigNum, siginfo_t* signalInfo, void* userContext) KSLOG_DEBUG("Trapped signal %d", sigNum); if(g_isEnabled) { - ksmc_suspendEnvironment(); + thread_act_array_t threads = NULL; + mach_msg_type_number_t numThreads = 0; + ksmc_suspendEnvironment(&threads, &numThreads); kscm_notifyFatalExceptionCaptured(false); KSLOG_DEBUG("Filling out context."); @@ -105,7 +107,7 @@ static void handleSignal(int sigNum, siginfo_t* signalInfo, void* userContext) crashContext->stackCursor = &g_stackCursor; kscm_handleException(crashContext); - ksmc_resumeEnvironment(); + ksmc_resumeEnvironment(threads, numThreads); kscm_innerHandleSignal(signalInfo); kscm_handleSignal(signalInfo); } diff --git a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_User.c b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_User.c index 082613e37..37be107ad 100644 --- a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_User.c +++ b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Monitors/KSCrashMonitor_User.c @@ -55,9 +55,11 @@ void kscm_reportUserExceptionSelfDefinePath(const char* name, } else { + thread_act_array_t threads = NULL; + mach_msg_type_number_t numThreads = 0; if(logAllThreads) { - ksmc_suspendEnvironment(); + ksmc_suspendEnvironment(&threads, &numThreads); } if(terminateProgram) { @@ -91,7 +93,7 @@ void kscm_reportUserExceptionSelfDefinePath(const char* name, if(logAllThreads) { - ksmc_resumeEnvironment(); + ksmc_resumeEnvironment(threads, numThreads); } if(terminateProgram) { @@ -114,9 +116,11 @@ void kscm_reportUserException(const char* name, } else { + thread_act_array_t threads = NULL; + mach_msg_type_number_t numThreads = 0; if(logAllThreads) { - ksmc_suspendEnvironment(); + ksmc_suspendEnvironment(&threads, &numThreads); } if(terminateProgram) { @@ -149,7 +153,7 @@ void kscm_reportUserException(const char* name, if(logAllThreads) { - ksmc_resumeEnvironment(); + ksmc_resumeEnvironment(threads, numThreads); } if(terminateProgram) { diff --git a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Tools/KSMachineContext.c b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Tools/KSMachineContext.c index 594f323cc..c266e131c 100644 --- a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Tools/KSMachineContext.c +++ b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Tools/KSMachineContext.c @@ -47,9 +47,6 @@ static KSThread g_reservedThreads[10]; static int g_reservedThreadsMaxIndex = sizeof(g_reservedThreads) / sizeof(g_reservedThreads[0]) - 1; static int g_reservedThreadsCount = 0; -static thread_act_array_t g_suspendedThreads = NULL; -static mach_msg_type_number_t g_suspendedThreadsCount = 0; - static inline bool isStackOverflow(const KSMachineContext* const context) { @@ -167,7 +164,7 @@ static inline bool isThreadInList(thread_t thread, KSThread* list, int listCount } #endif -void ksmc_suspendEnvironment() +void ksmc_suspendEnvironment(thread_act_array_t *suspendedThreads, mach_msg_type_number_t *numSuspendedThreads) { #if KSCRASH_HAS_THREADS_API KSLOG_DEBUG("Suspending environment."); @@ -175,15 +172,15 @@ void ksmc_suspendEnvironment() const task_t thisTask = mach_task_self(); const thread_t thisThread = (thread_t)ksthread_self(); - if((kr = task_threads(thisTask, &g_suspendedThreads, &g_suspendedThreadsCount)) != KERN_SUCCESS) + if((kr = task_threads(thisTask, suspendedThreads, numSuspendedThreads)) != KERN_SUCCESS) { KSLOG_ERROR("task_threads: %s", mach_error_string(kr)); return; } - for(mach_msg_type_number_t i = 0; i < g_suspendedThreadsCount; i++) + for(mach_msg_type_number_t i = 0; i < *numSuspendedThreads; i++) { - thread_t thread = g_suspendedThreads[i]; + thread_t thread = (*suspendedThreads)[i]; if(thread != thisThread && !isThreadInList(thread, g_reservedThreads, g_reservedThreadsCount)) { if((kr = thread_suspend(thread)) != KERN_SUCCESS) @@ -198,7 +195,7 @@ void ksmc_suspendEnvironment() #endif } -void ksmc_resumeEnvironment() +void ksmc_resumeEnvironment(thread_act_array_t threads, mach_msg_type_number_t numThreads) { #if KSCRASH_HAS_THREADS_API KSLOG_DEBUG("Resuming environment."); @@ -206,15 +203,15 @@ void ksmc_resumeEnvironment() const task_t thisTask = mach_task_self(); const thread_t thisThread = (thread_t)ksthread_self(); - if(g_suspendedThreads == NULL || g_suspendedThreadsCount == 0) + if(threads == NULL || numThreads == 0) { KSLOG_ERROR("we should call ksmc_suspendEnvironment() first"); return; } - for(mach_msg_type_number_t i = 0; i < g_suspendedThreadsCount; i++) + for(mach_msg_type_number_t i = 0; i < numThreads; i++) { - thread_t thread = g_suspendedThreads[i]; + thread_t thread = threads[i]; if(thread != thisThread && !isThreadInList(thread, g_reservedThreads, g_reservedThreadsCount)) { if((kr = thread_resume(thread)) != KERN_SUCCESS) @@ -225,13 +222,11 @@ void ksmc_resumeEnvironment() } } - for(mach_msg_type_number_t i = 0; i < g_suspendedThreadsCount; i++) + for(mach_msg_type_number_t i = 0; i < numThreads; i++) { - mach_port_deallocate(thisTask, g_suspendedThreads[i]); + mach_port_deallocate(thisTask, threads[i]); } - vm_deallocate(thisTask, (vm_address_t)g_suspendedThreads, sizeof(thread_t) * g_suspendedThreadsCount); - g_suspendedThreads = NULL; - g_suspendedThreadsCount = 0; + vm_deallocate(thisTask, (vm_address_t)threads, sizeof(thread_t) * numThreads); KSLOG_DEBUG("Resume complete."); #endif diff --git a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Tools/KSMachineContext.h b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Tools/KSMachineContext.h index 87f18c3a8..35208c04a 100644 --- a/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Tools/KSMachineContext.h +++ b/matrix/matrix-iOS/Matrix/WCCrashBlockMonitor/KSCrash/Recording/Tools/KSMachineContext.h @@ -34,14 +34,15 @@ extern "C" { #include "KSThread.h" #include +#include /** Suspend the runtime environment. */ -void ksmc_suspendEnvironment(void); +void ksmc_suspendEnvironment(thread_act_array_t *suspendedThreads, mach_msg_type_number_t *numSuspendedThreads); /** Resume the runtime environment. */ -void ksmc_resumeEnvironment(void); +void ksmc_resumeEnvironment(thread_act_array_t threads, mach_msg_type_number_t numThreads); /** Create a new machine context on the stack. * This macro creates a storage object on the stack, as well as a pointer of type