Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/#review58915 --- The GarbageCollector is one of the earliest libprocess processes that was added. It doesn't follow the preferred 'pimpl' style where we should have both a GarbageCollectorProcess wrapped (via an Owned) in a GarbageCollector, which controls the lifetime via spawn, terminate, and wait. I'd prefer to see us fix this and then simply allocate and free an instance of GarbageCollector. Make sense? - Benjamin Hindman On Oct. 22, 2014, 6:28 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 22, 2014, 6:28 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 22, 2014, 6:28 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/#review57870 --- Bad patch! Reviews applied: [26712, 26578, 26580, 26583] Failed command: ./support/apply-review.sh -n -r 26583 Error: --2014-10-22 19:35:17-- https://reviews.apache.org/r/26583/diff/raw/ Resolving reviews.apache.org (reviews.apache.org)... 140.211.11.74 Connecting to reviews.apache.org (reviews.apache.org)|140.211.11.74|:443... connected. HTTP request sent, awaiting response... 200 OK Length: 1052 (1.0K) [text/x-patch] Saving to: '26583.patch' 0K . 100% 50.2M=0s 2014-10-22 19:35:17 (50.2 MB/s) - '26583.patch' saved [1052/1052] error: patch failed: 3rdparty/libprocess/src/process.cpp:1690 error: 3rdparty/libprocess/src/process.cpp: patch does not apply Failed to apply patch - Mesos ReviewBot On Oct. 22, 2014, 6:28 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 22, 2014, 6:28 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
On Oct. 10, 2014, 5:15 p.m., Dominic Hamon wrote: 3rdparty/libprocess/src/process.cpp, line 1631 https://reviews.apache.org/r/26583/diff/1/?file=717970#file717970line1631 this can't land until https://reviews.apache.org/r/26289/ which contains the configure.ac check for std::unique_ptr support. Dominic, what is the expected time frame for landing r26289? It contains more than just the check and we have a couple of patches that would depend on it. Can we pull out the check in a new patch to get going? - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/#review56262 --- On Oct. 10, 2014, 4:12 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 10, 2014, 4:12 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
On Oct. 10, 2014, 5:15 p.m., Dominic Hamon wrote: 3rdparty/libprocess/src/process.cpp, line 1631 https://reviews.apache.org/r/26583/diff/1/?file=717970#file717970line1631 this can't land until https://reviews.apache.org/r/26289/ which contains the configure.ac check for std::unique_ptr support. Niklas Nielsen wrote: Dominic, what is the expected time frame for landing r26289? It contains more than just the check and we have a couple of patches that would depend on it. Can we pull out the check in a new patch to get going? just split it out: https://reviews.apache.org/r/26712/ - Dominic --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/#review56262 --- On Oct. 10, 2014, 4:12 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 10, 2014, 4:12 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
On Oct. 11, 2014, 12:15 a.m., Dominic Hamon wrote: 3rdparty/libprocess/src/process.cpp, line 1635 https://reviews.apache.org/r/26583/diff/1/?file=717970#file717970line1635 should do the same for this while you're here. Since help is covered by the garbage collector it doesn't make sense to wrap it with a unique_ptr; otherwise we would have to .release() it to prevent a double free. - Joris --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/#review56262 --- On Oct. 10, 2014, 11:12 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 10, 2014, 11:12 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Review Request 26583: Memory cleanup: libprocess delete garbage collector process
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/#review56261 --- Bad patch! Reviews applied: [26578, 26580, 26583] Failed command: git apply --index 26583.patch Error: error: patch failed: 3rdparty/libprocess/src/process.cpp:1690 error: 3rdparty/libprocess/src/process.cpp: patch does not apply - Mesos ReviewBot On Oct. 10, 2014, 11:12 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 10, 2014, 11:12 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 26583: Memory cleanup: libprocess delete garbage collector process
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/#review56262 --- 3rdparty/libprocess/src/process.cpp https://reviews.apache.org/r/26583/#comment96565 this can't land until https://reviews.apache.org/r/26289/ which contains the configure.ac check for std::unique_ptr support. 3rdparty/libprocess/src/process.cpp https://reviews.apache.org/r/26583/#comment96564 should do the same for this while you're here. - Dominic Hamon On Oct. 10, 2014, 4:12 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26583/ --- (Updated Oct. 10, 2014, 4:12 p.m.) Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: mesos-git Description --- Wrap the libprocess garbage collector process with a unique ptr. Reset the ptr during finalization. Diffs - 3rdparty/libprocess/src/process.cpp 85fb995 Diff: https://reviews.apache.org/r/26583/diff/ Testing --- make check Thanks, Joris Van Remoortere
