> On Aug. 13, 2016, 9:13 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/process.cpp, line 413
> > <https://reviews.apache.org/r/40266/diff/7/?file=1472251#file1472251line413>
> >
> >     `all` is a bit misleading since "gc" is not being terminated. can you 
> > just call it "terminate"? i know there is already a process specific 
> > overload. or maybe call it "finalize" ?

I renamed this earlier to remove the semantic between 
`ProcessManager::finalize` and `process::finalize`.
See: https://reviews.apache.org/r/40266/#comment194260

I can call it `terminate_all_except_gc` to be clearer.


> On Aug. 13, 2016, 9:13 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1143
> > <https://reviews.apache.org/r/40266/diff/7/?file=1472251#file1472251line1143>
> >
> >     but all the processes are already terminated by this point? do you mean 
> > just the gc process my dereference it?

Even `gc` will be terminated in a codepath that hits `socket_manager`.  And new 
processes *may* be created between terminating everything (but gc) and cleaning 
up the socket manager.


> On Aug. 13, 2016, 9:13 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/process.cpp, line 1170
> > <https://reviews.apache.org/r/40266/diff/7/?file=1472251#file1472251line1170>
> >
> >     but process manager is cleaned up above?

`__address__` is mostly used to distinguish local and remote PIDs.


> On Aug. 13, 2016, 9:13 p.m., Vinod Kone wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 2410-2417
> > <https://reviews.apache.org/r/40266/diff/7/?file=1472251#file1472251line2410>
> >
> >     why do this here instead of #1155?

The `processes_mutex` is only available inside the ProcessManager.  We need to 
synchronize the delete of `gc` as an asynchronous call to `process::spawn(..., 
true)` during finalization may try to spawn a gc-managed process.


- Joseph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40266/#review145715
-----------------------------------------------------------


On Sept. 19, 2016, 5:26 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40266/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2016, 5:26 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Joris Van Remoortere, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3910
>     https://issues.apache.org/jira/browse/MESOS-3910
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `SocketManager` and `ProcessManager` are highly inter-dependent, 
> which requires some untangling in `process::finalize`.
> 
> * Logic originally found in `~ProcessManager` has been split into 
>   `ProcessManager::finalize` due to what happens during cleanup.
> * The future from `__s__->accept()` must be explicitly discarded as 
>   libevent does not detect a locally closed socket.
> * Terminating `HttpProxy`s must close the associated socket.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp 
> a6fbf92bd6d69d94490b9faf4960f14aa112e8d2 
> 
> Diff: https://reviews.apache.org/r/40266/diff/
> 
> 
> Testing
> -------
> 
> `make check` (libev)
> `make check` (--enable-libevent --enable-ssl)
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to