----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62053/#review184735 -----------------------------------------------------------
Looks good, just curious why you had to change the semantics of moving the process ownership. Seems worse than before since there's a special case where the caller has to perform the delete if spawn returns `UPID()`? 3rdparty/libprocess/include/process/process.hpp Lines 574-575 (patched) <https://reviews.apache.org/r/62053/#comment260934> Why did you change the semantics here? Previously, this seemed easy to understand since you're either moving ownership or not. Now, when you're moving ownership there is a special case that you always have to deal with? 3rdparty/libprocess/src/process.cpp Lines 2658-2659 (original), 2651-2652 (patched) <https://reviews.apache.org/r/62053/#comment260933> Do we need to give them an async wait? Why is exited not equivalent to that? 3rdparty/libprocess/src/process.cpp Lines 3404 (patched) <https://reviews.apache.org/r/62053/#comment260931> Do you want to say that the invariant right now is that a process always goes through initialization before cleanup? Seems like that's something we might change later (e.g. no point initializing if we already want to terminate it?) 3rdparty/libprocess/src/process.cpp Line 3484 (original), 3458 (patched) <https://reviews.apache.org/r/62053/#comment260930> Can you clarify that it's because a user could rely exited events to know when to delete? Per my comment above, I'm curious why that's wrong. - Benjamin Mahler On Sept. 2, 2017, 3:07 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62053/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2017, 3:07 p.m.) > > > Review request for mesos, Benjamin Mahler and Jiang Yan Xu. > > > Bugs: MESOS-7921 > https://issues.apache.org/jira/browse/MESOS-7921 > > > Repository: mesos > > > Description > ------- > > The garbage collector had at least two bugs: > > (1) If someone dispatched `manage()` twice in a row the process we're > waiting for will get overwritten which can wreak havoc depending > on when the calls to `link()` happen. > > (2) The garbage collector was deleting after an exited event rather > than actually doing a `wait()`. > > The simpler implementation that this patch introduces is to just > delete the process after doing `ProcessManager::cleanup()`. > > > Diffs > ----- > > 3rdparty/libprocess/include/Makefile.am > c5dc0bb0d2d77987531ead50277940700c62b84f > 3rdparty/libprocess/include/process/gc.hpp > 603bb8bb084a8d2774ab1077da671f659a22a376 > 3rdparty/libprocess/include/process/process.hpp > 8cca782ae89727bc5570afc4ed96c556f14c8c68 > 3rdparty/libprocess/src/process.cpp > afa53537a5c7d4d8b0f4e3b8e04d7d0f2c4c6631 > 3rdparty/libprocess/src/tests/process_tests.cpp > 8d36600701a795a7fa8d73a844657ff98eee6aa7 > > > Diff: https://reviews.apache.org/r/62053/diff/2/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >
