Page MenuHomePhabricator

Implement ProcessLauncherWindows, and refactor some other code to be more modular.

Authored by zturner on Oct 14 2014, 12:45 PM.



This implements Host::LaunchProcess for windows, and in doing so does some minor refactor to move towards a more modular process launching design, as discussed in detail previously on the list.

The original motivation for this is that launching processes on windows needs some very windows specific code, which would live most appropriately in source/Host/windows somewhere. However, there is already some common code that all platforms use when launching a process before delegating to the platform specific stuff, which lives in source/Host/common/Host.cpp which would be nice to reuse without duplicating.

This commonality has been abstracted into MonitoringProcessLauncher, a class which abstracts out the notion of launching a process using an arbitrary algorithm, and then monitoring it for state changes.

The windows specific launching code lives in ProcessLauncherWindows, and the posix specific launching code lives in ProcessLauncherPosix. after the refactor, a MonitoringProcessLauncher is created, and then an appropriate delegate launcher is created and given to the MonitoringProcessLauncher.

Comments have been inserted to indicate future places for cleanup, so that this refactor can happen iteratively instead as one massive risky CL.

Tested on Windows, MacOSX, and Linux. Did not see any test regressions.

Diff Detail


Event Timeline

zturner updated this revision to Diff 14880.Oct 14 2014, 12:45 PM
zturner retitled this revision from to Implement ProcessLauncherWindows, and refactor some other code to be more modular..
zturner updated this object.
zturner edited the test plan for this revision. (Show Details)
zturner added reviewers: clayborg, jingham.
zturner added a subscriber: Unknown Object (MLST).
clayborg requested changes to this revision.Oct 14 2014, 1:45 PM
clayborg edited edge metadata.

In Host.cpp please initialize result_pid right away just in case to make sure we don't ever try to use a bogus value. Other than that, it looks good.

This revision now requires changes to proceed.Oct 14 2014, 1:45 PM
zturner closed this revision.Oct 14 2014, 3:05 PM
zturner updated this revision to Diff 14896.

Closed by commit rL219731 (authored by @zturner).