[Lldb-commits] [PATCH] Add code to exit the NativeProcessLinux Monitor thread on android
tberghammer at google.com
Thu Mar 12 06:30:11 PDT 2015
Comment at: source/Host/common/Host.cpp:146
@@ +145,3 @@
+// Change it to a std::set with a mutex if multiple thread can be cancelled at the same time
+static pthread_t g_usr1_signal_thread = 0;
> labath wrote:
> > Please remove the std::set reference, as it is not a valid solution. There are very few things that can be done safely in a signal handler and std::set manipulation is not one of them. If we ever need to cancel multiple threads, we will need to do something more clever, probably set a flag on NativeProcessAndroid (outside of the signal handler), as @vharron suggseted.
> It just occurred to me that this would be a good use case for a thread_local variable. I'm not sure what is the official policy on thread local variables, as I can't find any occurrences in the codebase, but if that's ok then the killer solution would be
> static thread_local volatile sig_atomic_t g_usr1_called;
> If not then you can at least make your existing variable volatile for some added safety.
Comment at: source/Host/common/Host.cpp:194
@@ -176,2 +193,3 @@
// Wait for all child processes
-#if !defined(__ANDROID__) && !defined(__ANDROID_NDK__)
+ if (g_usr1_signal_thread == ::pthread_self())
> I'd rather put this snippet into a separate function since it's called twice:
> bool CheckForCancellation()
> #ifdef __linux__
> if (g_usr1_signal_thread == ::pthread_self())
> g_usr1_signal_thread = 0;
> return true;
> return false;
> ::pthread_testcancel ();
Comment at: source/Host/common/Host.cpp:262
@@ -232,3 +261,3 @@
log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS);
> Unrelated but looks weird - could you remove this log re-initialization? It looks unnecessary.
It is necessary to let the user enable the logging after starting the monitor thread.
More information about the lldb-commits