Make it possible to stop the operation thread in NativeProcessLinux

[tberghammer]

Summary

Make it possible to stop the operation thread in NativeProcessLinux

Previously the operation thread is stopped with a cancel event but pthread_cancel is not supported on android. This CL creates a custom operation which asks the operation thread to exit without any pthread call.

[emaste]

I think it's worth adding a comment in the source explaining why .Cancel() is not used.

[ovyalov]

Looks good - please address suggestions before submitting.

source/Plugins/Process/Linux/NativeProcessLinux.cpp

3566 ↗

(On Diff #20846)

Could you also swap StopOpThread() and StopCoordinatorThread ()?
If there there are en-queued and/or active commands in TSC thread that need PTRACE they will deadlock on DoOperation since operation thread has been stopped.

[ovyalov]

lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp

3568

Actually, I meant such sequence:

StopMonitoringChildProcess(); // en-queues PTRACE & TSC commands 
StopCoordinatorThread (); // en-queues PTRACE commands
StopOpThread();

In this case, StopOpThread is executed when there is nobody around who can issue a PTRACE request.

[ovyalov]

This CL seems to make deadlock on TestAttachDenied.py - DoOperation(nullptr) is blocked on "while (sem_wait(&m_operation_done))" because attach operation failed and operation thread has exited already. I assume the issue is also happening if launch fails. If attach/launch fails we need just join without calling DoOperation.

[tberghammer]

Thanks for pointing out the deadlock.
Address both issue with D8030.

lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp

for(;;)
         // wait for next pending operation
         if (sem_wait(&monitor->m_operation_pending))
         {
             if (errno == EINTR)
                 continue;
             assert(false && "Unexpected errno from sem_wait");
         }
 
+        // nullptr as operation means the operation thread should exit. Cancel() can't be used
+        // because it is not supported on android.
+        if (!monitor->m_operation)
+        {
+            // notify calling thread that operation is complete
+            sem_post(&monitor->m_operation_done);
+            break;
+        }
+
         reinterpret_cast<Operation*>(monitor->m_operation)->Execute(monitor);
 
         // notify calling thread that operation is complete
         sem_post(&monitor->m_operation_done);
     }
 }
 
 void
if (m_monitor_thread.IsJoinable())
         m_monitor_thread.Cancel();
         m_monitor_thread.Join(nullptr);
     }
 }
 
 void
 NativeProcessLinux::StopMonitor()
 {
-    StopMonitoringChildProcess();
     StopOpThread();
+    StopMonitoringChildProcess();
     StopCoordinatorThread ();
     sem_destroy(&m_operation_pending);
     sem_destroy(&m_operation_done);
 
     // TODO: validate whether this still holds, fix up comment.
     // Note: ProcessPOSIX passes the m_terminal_fd file descriptor to
     // Process::SetSTDIOFileDescriptor, which in turn transfers ownership of
     // the descriptor to a ConnectionFileDescriptor object.  Consequently
     // even though still has the file descriptor, we shouldn't close it here.
 }
 
 void
 NativeProcessLinux::StopOpThread()
 {
     if (!m_operation_thread.IsJoinable())
         return;
 
-    m_operation_thread.Cancel();
+    DoOperation(nullptr); // nullptr as operation ask the operation thread to exit
     m_operation_thread.Join(nullptr);
 }
 
 Error
 NativeProcessLinux::StartCoordinatorThread ()
 {
     Error error;
     static const char *g_thread_name = "lldb.process.linux.ts_coordinator";