Refactor ProcessSpawn
There are a few things that could be improved in the ProcessSpawn implementation. They become more apparent now that the synchronous functionality has been added to it, but the issues were present before too.
- The asynchronous implementation of ProcessSpawn relies on having a global IO signal set and on having the IO service being periodically polled in order to wait for child processes which is why it uses the main IO service. For this reason:
- There needs to be a dedicated AsyncProcessSpawn class that should be a singleton to signal to the developer that it has global dependency objects.
- There needs to be a method in AsyncProcessSpawn that sets the IO service. It needs to be callable only once and called as close as possible to the creation of the main IO service. Spawning would throw if the IO service is not initialized. This is to avoid the current behavior which sets the IO service on the first ProcessSpawn creation which could be on a null IOServicePtr. See
src/hooks/dhcp/run_script/run_script.cc
orsrc/hooks/dhcp/forensic_log/rotating_file.cc
.
- There should be a new class encapsulating the synchronous implementation, say
SyncProcessSpawn
. It does not have to be a singleton since waiting for the child process is done in the scope it was declared, and the object can be safely deleted afterwards. - It is worth considering to change the sync variant to use an IO signal set and an IO service like the async variant, but these should not be global, but declarable by the developer, or even better, hidden by the implementation.
- The dismiss feature in spawn is not used in code. I suggest removing it.
- The async process spawn is currently fire-and-forget. It would be nice for the async process spawn to have the ability to be notified that the process has finished and that its status is available. Maybe with the help of a condition variable?