Launch lldb-gdbserver in same process group when launched remotely using lldb-platform.

[flackr]

Summary

When launched from lldb-platform, lldb-gdbserver's lifetime should be tied to the lldb-platform process. Otherwise the platform can die and leave a lingering lldb-gdbserver process. The existing launch argument LaunchInSeparateProcessGroup is used to denote this as this seems to be exactly what it is intended for.

[zturner]

Looks fine to me. But I'm neither a linux person nor an llgs person, so I'm fine with it as long as Siva and Oleksiy are.

[ovyalov]

Do we need to call SetLaunchInSeparateProcessGroup(true) here https://github.com/llvm-mirror/lldb/blob/0d9ea6ba74541fe4a73df96122dc7a5913ac91e5/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp#L1918?

[flackr]

In D7211#114246, @ovyalov wrote:

Do we need to call SetLaunchInSeparateProcessGroup(true) here https://github.com/llvm-mirror/lldb/blob/0d9ea6ba74541fe4a73df96122dc7a5913ac91e5/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp#L1918?

No, this is how the server is launched when run from lldb-platform, where we don't want it to run in a separate process group. Note the m_is_platform check here:
https://github.com/llvm-mirror/lldb/blob/0d9ea6ba74541fe4a73df96122dc7a5913ac91e5/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp#L1887

I could call SetLaunchInSeparateProcessGroup(false) to make that explicit.

[ovyalov]

Do we need to call SetLaunchInSeparateProcessGroup(true) here https://github.com/llvm-mirror/lldb/blob/0d9ea6ba74541fe4a73df96122dc7a5913ac91e5/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp#L1918?

In D7211#114251, @flackr wrote:

In D7211#114246, @ovyalov wrote:

Do we need to call SetLaunchInSeparateProcessGroup(true) here https://github.com/llvm-mirror/lldb/blob/0d9ea6ba74541fe4a73df96122dc7a5913ac91e5/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp#L1918?

No, this is how the server is launched when run from lldb-platform, where we don't want it to run in a separate process group. Note the m_is_platform check here:
https://github.com/llvm-mirror/lldb/blob/0d9ea6ba74541fe4a73df96122dc7a5913ac91e5/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp#L1887

I could call SetLaunchInSeparateProcessGroup(false) to make that explicit.

Yes, I think it makes sense to do - otherwise we may rely on default value of LaunchInSeparateProcessGroup.

[flackr]

Explicitly set LaunchInSeparateProcessGroup to false from GDBRemoteCommunicationServer::Handle_qLaunchGDBServer to make this explicit and not rely on default value.

[flackr]

I don't have commit access so if/when this is good can someone land it for me? Thanks.

[ovyalov]

Submitted as http://reviews.llvm.org/rL227329

[flackr]

Committed in http://reviews.llvm.org/rL227329

source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp

         }
 
         // use native registers, not the GDB registers
-        debugserver_args.AppendArgument("--native-regs");   
-        // make debugserver run in its own session so signals generated by 
-        // special terminal key sequences (^C) don't affect debugserver
-        debugserver_args.AppendArgument("--setsid");
+        debugserver_args.AppendArgument("--native-regs");
+
+        if (launch_info.GetLaunchInSeparateProcessGroup())
+        {
+            debugserver_args.AppendArgument("--setsid");
+        }
 
         char named_pipe_path[PATH_MAX];
         named_pipe_path[0] = '\0';

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp

         if (log)
             log->Printf("Launching debugserver with: %s:%u...\n", hostname.c_str(), port);
 
+        // Do not run in a new session so that it can not linger after the
+        // platform closes.
+        debugserver_launch_info.SetLaunchInSeparateProcessGroup(false);
         debugserver_launch_info.SetMonitorProcessCallback(ReapDebugserverProcess, this, false);
 
         std::string platform_scheme;

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

         static FileSpec g_debugserver_file_spec;
 
         ProcessLaunchInfo debugserver_launch_info;
+        // Make debugserver run in its own session so signals generated by
+        // special terminal key sequences (^C) don't affect debugserver.
+        debugserver_launch_info.SetLaunchInSeparateProcessGroup(true);
+
         debugserver_launch_info.SetMonitorProcessCallback (MonitorDebugserverProcess, this, false);
         debugserver_launch_info.SetUserID(process_info.GetUserID());