mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ben Mahler" <benjamin.mah...@gmail.com>
Subject Re: Review Request 17306: Added an asynchronous subprocess utility.
Date Wed, 29 Jan 2014 22:39:48 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17306/
-----------------------------------------------------------

(Updated Jan. 29, 2014, 10:39 p.m.)


Review request for mesos, Benjamin Hindman, Ian Downes, Jie Yu, and Vinod Kone.


Changes
-------

Refactored to return a Try<Subprocess> and ensured we close pipes when returning an
Error.
Moved the pipe closing code into a Data destructor which cleans up the code a bit.


Bugs: MESOS-943
    https://issues.apache.org/jira/browse/MESOS-943


Repository: mesos-git


Description
-------

This adds an asynchronous mechanism for subprocess execution, per MESOS-943.

What started simple was made a little more complex due to the following issues:

1. Who is responsible for closing the input / output descriptors?

   Placing this burden onto the caller of subprocess() seems likely to yield leaked open file
descriptors. This introduced the notion of a shared_ptr / destructor / copy constructor /
assignment constructor to ensure that the file descriptors are closed when the handle to the
file descriptors are lost. However, even with my implementation, one may copy these file descriptors,
at which point they may be deleted from underneath them.

2. What does discarding the status entail? Does it kill the process?

   The current implementation kills the process, which requires the use of an explicit Promise
to deal with the discard from the caller not affecting the reaper's future. If discard() is
a no-op, we must still use an explicit Promise to preserve the notification from the Reaper
(so that we can know when to delete the Reaper).


That's about it, I've added tests that demonstrate the ability to communicate with the subprocess
through stdin / stout / stderr.

Please let me know if you find any simplifications that can be made! (Other than C++11 lambdas,
of course :))


Diffs (updated)
-----

  3rdparty/libprocess/Makefile.am 40f01a7b3803696ccca440c8326e1d6d7c377459 
  3rdparty/libprocess/include/process/subprocess.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/17306/diff/


Testing
-------

Tests were added and ran in repetition.


Thanks,

Ben Mahler


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message