> On April 5, 2016, 12:52 p.m., Joseph Wu wrote: > > src/tests/cluster.cpp, lines 364-366 > > <https://reviews.apache.org/r/45689/diff/1/?file=1324705#file1324705line364> > > > > The destructor will only dereference a null `containerizer` if this > > error case is hit (or if you pass in a `nullptr` cast to > > `slave::Containerizer*`). > > > > At this point in the code, we haven't done anything other than create > > some objects. So it should be ok to do something like this in the > > destructor: > > ``` > > if (containerizer == nullptr) { > > return; > > } > > ``` > > The call to `terminate()` only matters if the `cluster::Slave` is > > created successfully.
I agree that Haosdent's suggestion works for the purpose of this fix. But if "The call to terminate() only matters if the cluster::Slave is created successfully", then we should call terminate() under the condition that `pid` is not empty in `~Slave()`. We should fix it in a separate review. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45689/#review127195 ----------------------------------------------------------- On April 4, 2016, 10:31 a.m., James Peach wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45689/ > ----------------------------------------------------------- > > (Updated April 4, 2016, 10:31 a.m.) > > > Review request for mesos, Joris Van Remoortere, Joseph Wu, and Jiang Yan Xu. > > > Repository: mesos > > > Description > ------- > > If Cluster::Slave::start() fails, make sure we don't crash in the > destructor. > > > Diffs > ----- > > src/tests/cluster.cpp 14d0d34fcb4c408ad996672394c39c84fd2be918 > > Diff: https://reviews.apache.org/r/45689/diff/ > > > Testing > ------- > > Make check. > > > Thanks, > > James Peach > >
