<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 24, 2014 at 10:56 AM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Comments:<br>
<br>
- When running processes one might want to get signals for the process that are delivered while the child process is running. There can be any amount of signals over the child process' lifetime. This must be included in the design. It might not work for windows, but it will need to be worked into the design.<br></blockquote><div><br></div><div>Can you clarify? The goal was to propose this in such a way that it's functionally equivalent to the current code. In the existing code, what is the means by which this currently happens? There is the ability to send a signal <b>to</b> a process, and that is covered in this design by providing a Signal() method on HostNativeProcessPosix. Anyone calling that method is invoking platform specific functionality, so they would have either already be inside platform specific code, or write something like HostProcess->GetNativeProcess()->Signal() wrapped in an appropriate #ifdef.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- You can change the internal API, but you can't change the public API. If you make changes to any of the launch info classes internally, you will need to keep the SBLaunchInfo API the same. I know you like cleaning things up, but we have clients that use this API and we don't want it broken. You will need to carefully check for any other things that get modified in the launch info class and make sure any such cases are fixed.<br></blockquote><div>This shouldn't involve a change to the public API.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- We should make ProcessLauncher a plug-in and be able to request them by name. Then we can add things to the launch info to be able to specify the process launcher plug-in name ("posix_spawn", "fork", "apple-xpc", "default" which can map to the default launcher for a host, etc).<br></blockquote><div>Good idea, probably can be done as a last step. All of the actual logic would still live in Host, since it's OS-specific, and the plugin code could just be a lightweight shim to select the correct one.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">><br>
> This approach yields a number of benefits. For starters, it allows us to abstract out the notion of the launching strategy into a reusable piece of code so that platforms which have multiple launching strategies have the ability to make use of specific strategies in other platform specific code, or so that remote process launches can be treated the same as local process launches. As an example, on MacOSX there are three different launching strategies: Launch with XPC services, launch with Applescript, and launch with posix_spawn. Each of these could be independent implementations of |ProcessLauncher|, with the default implementation on MacOSX being a fourth implementation that chooses one of the first 3 implementations to delegate to at runtime. As a result, the 3 implementations can be used independently of each other should the need arise.<br>
><br>
> A second benefit is that it opens up the possibility of treating remote process launch transparently with local process launch -- just have a |RemoteProcessLauncher| class.<br>
<br>
</span>Uh... Then don't call your class HostProcess. I really would rather not have remote launching involved in this at all. Remote launching should result in a lldb::pid_t and it won't use the native process layer for anything, so I would avoid including remote process launching in this as it mucks up what info might need to be in the launch info (remote host/port, password, etc). So no remote process support please.<br></blockquote><div>Fair enough, I mentioned that just as something that may or may not be useful in the future, but the design lends itself nicely to. I don't want to choose names based on something that might not happen though, so I think it still makes sense to go with HostProcess, since for now that's what it actually is, and gives some naming consistency between this and HostThread.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
><br>
> There are additional benefits in the form of overall code health, platform separation, and just generally improving the modularity of the codebase, although those are perhaps less tangible.<br>
><br>
> Monitoring for process exit<br>
> It is important to know when a process exits, for 2 reasons. First, on some platforms the process needs to be reaped. Second, asynchronous exit callbacks need to be invoked. The implementation of the monitoring is platform dependent, and a strategy for each platform can be summarized as follows:<br>
><br>
> • Windows<br>
> • Create a background thread, use WaitForSingleObject<br>
> • [Better, alternative implementation] Create one "static" background thread for all processes, use WaitForMultipleObjects() to monitor many processes at once, like select().<br>
> • MacOSX<br>
> • Processes spawned with posix_spawn - Use MacOSX "dispatch" library to queue monitoring code on a system thread.<br>
> • Processes spawned with XPC - Use MacOSX "dispatch" library to queue monitoring code on a system thread.<br>
> • Processes spawned with Applescript - No monitoring necessary<br>
> • Non-MacOSX posix systems<br>
> • Processes spawned with posix_spawn - Create a background thread, use waitpid in the background thread.<br>
<br>
</span>We also need to be able to deliver signals during child process execution.<br></blockquote><div>Mentioned earlier, but this is already covered. HostProcess is a wrapper around a HostNativeProcess, and provides a method just like HostThread does, which returns the native process. Since signal is not a generic operation, it wouldn't live on HostProcess, but on HostNativeProcessPosix. Anyone wishing to deliver a signal to a child process would write host_process.GetNativeProcess()->Signal(SIGTERM) or whatever. If they write this from generic code it would need to be inside of an #ifdef. If they write it from platform-specific code it will just work.</div><div><br></div></div></div></div>