----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56195/#review164040 -----------------------------------------------------------
src/slave/containerizer/mesos/containerizer.cpp (line 1140) <https://reviews.apache.org/r/56195/#comment235572> Hum, this could cause as FD being closed twice. Say `_launch` fails after `launcher->fork`. That means the subprocess call already close the parent end of the pipe. Here, you'll be closing it again. I think the only elegant fix is to introduce file descriptor abstraction and use RAII to make sure the fd will be closed in error case. - Jie Yu On Feb. 2, 2017, 5:29 a.m., Kevin Klues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56195/ > ----------------------------------------------------------- > > (Updated Feb. 2, 2017, 5:29 a.m.) > > > Review request for mesos, Alexander Rukletsov, Gastón Kleiman, Gilbert Song, > and Jie Yu. > > > Bugs: MESOS-7050 > https://issues.apache.org/jira/browse/MESOS-7050 > > > Repository: mesos > > > Description > ------- > > Previously, if the containizer launch path failed before actually > launching the container, the FDs allocated to the container by the > IOSwitchboard isolator would be leaked. This would lead to deadlock in > the destroy path because the IOSwitchboard does not shutdown until the > FDs it allocates to the container have been closed. Since the > switchboard doesn't shutdown, the future returned by its 'cleanup()' > function is never satisfied. > > This commit makes sure to close the FDs under all failure cases in the > launch path. > > > Diffs > ----- > > src/slave/containerizer/mesos/containerizer.cpp > 4f0a773676da45fa40ad1ad9cdfab2a19249247d > > Diff: https://reviews.apache.org/r/56195/diff/ > > > Testing > ------- > > Linux CentOS 7: > ``` > GTEST_FILTER="" make -j check > src/mesos-tests > [----------] Global test environment tear-down > [==========] 1477 tests from 167 test cases ran. (390804 ms total) > [ PASSED ] 1477 tests. > ``` > > Mac OS X (El Capitan) > ``` > GTEST_FILTER="" make -j check > src/mesos-tests > [ FAILED ] 3 tests, listed below: > [ FAILED ] ExamplesTest.V1JavaFramework > [ FAILED ] ExamplesTest.PythonFramework > ``` > > > Thanks, > > Kevin Klues > >
