> On May 25, 2018, 5:12 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/grpc.cpp
> > Lines 70 (patched)
> > <https://reviews.apache.org/r/67157/diff/4/?file=2024986#file2024986line80>
> >
> > I am slightly worried that we only `join` this thread on `terminate`.
> > Could we make sure we `join` the thread whenever `looper` gets `reset` or
> > goes out of scope -- we might be able to e.g., use a custom deleter for
> > that if the lifetimes are guaranteed to be correct.
Maybe as a simpler solution, let's call `looper.reset()` here and add a
destructor
Runtime::RuntimeProcess::~RuntimeProcess()
{
CHECK(!looper);
}
to make sure on the language level (in addition to libprocess `Process`
semantics) that we never leak this thread.
> On May 25, 2018, 5:12 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/grpc.cpp
> > Lines 76 (patched)
> > <https://reviews.apache.org/r/67157/diff/4/?file=2024986#file2024986line86>
> >
> > This seems almost like the libprocess equivalent of a throwing
> > destructor. Could we trigger this e.g., if we fail a test `ASSERT`?
> >
> > Also see my comment regarding joining whenever `looper` goes out of
> > scope.
>
> Chun-Hung Hsiao wrote:
> Given that we never directly call `process::terminate` on a
> `RuntimeProcess`, but only indirectly terminate the process through
> `RuntimeProcess::terminate` which also sets `terminating`, this cannot fail
> at any case, including failed test assertions. I could remove this if this
> line looks weird to you.
Makes sense, let's leave the assertion in here.
Dropped
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67157/#review203871
-----------------------------------------------------------
On May 18, 2018, 12:23 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67157/
> -----------------------------------------------------------
>
> (Updated May 18, 2018, 12:23 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and
> Zhitao Li.
>
>
> Bugs: MESOS-8924
> https://issues.apache.org/jira/browse/MESOS-8924
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The refactoring does the following things:
> 1. Manage the gRPC completion queue and the looper thread in the runtime
> process to get rid of a lock in `Runtime::Data`.
> 2. Move the computation of sending a request into the runtime process.
> 3. Let libprocess manage the runtime process automatically instead of
> managing its lifecycle in `Runtime::Data`.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/grpc.hpp
> 321a46e19c69eafb24012bcef68bb8b0cc6aa436
> 3rdparty/libprocess/src/grpc.cpp a80bcb614ec96d92d21bc88a281d3208e86141a0
>
>
> Diff: https://reviews.apache.org/r/67157/diff/4/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
