Re: Review Request 57973: Windows: Added `JobObjectManager` actor.

[Joseph Wu <jo...@mesosphere.io>]

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


Ship it!




LGTM.  I can fix the below before committing.


3rdparty/libprocess/include/process/windows/jobobject.hpp
Lines 43 (patched)
<https://reviews.apache.org/r/57973/#comment243766>

    Whitespace:
    ```
      void manage(
          const pid_t pid, 
          const std::string& name, 
          const SharedHandle& handle)
    ```



3rdparty/libprocess/include/process/windows/jobobject.hpp
Lines 48 (patched)
<https://reviews.apache.org/r/57973/#comment243767>

    Whitespace:
    ```
          .onAny(defer([this, pid](const Future<Option<int>>& exit_code) {
             cleanup(exit_code, pid); 
           }));
    ```
    
    Although, my suggestion in a previous review also works:
    ```
      .onAny(defer(self(), &Self::cleanup, lambda::_1, pid));
    ```



3rdparty/libprocess/src/process.cpp
Lines 88-91 (original), 88-94 (patched)
<https://reviews.apache.org/r/57973/#comment243773>

    Alphabetical ordering, so we should insert this after the SSL header.


- Joseph Wu


On March 31, 2017, 4:36 p.m., Andrew Schwartzmeyer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57973/
> -----------------------------------------------------------
> 
> (Updated March 31, 2017, 4:36 p.m.)
> 
> 
> Review request for mesos, John Kordich, Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6868 and MESOS-6892
>     https://issues.apache.org/jira/browse/MESOS-6868
>     https://issues.apache.org/jira/browse/MESOS-6892
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This commit adds a Windows specific actor for managing job objects.
> 
> A subprocess launched with the `ParentHook::CREATE_JOB()` is created
> within the context of a named Windows job object. The `JobObjectManager`
> takes ownership of the handle to the job object.
> 
> It is necessary to tie the lifetime of the job object to the actor by
> ownership of the open handle so that the job object can be queried for
> usage information even after the processes that were running within the
> job object have ended. These semantics were not changed; previously the
> same was achieved by leaking the handle and tieing it to the lifetime of
> the actual Mesos agent process, and implicitly depending on the
> operating system to close the open handle at the death of the process.
> 
> We ensure the proper death of the job object process group by defering a
> call to `cleanup()` to the process reaper for the given PID. This
> function uses the Windows system call to terminate the job object via
> `os::kill_job()`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/windows/jobobject.hpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp f6ee24e2db43d63d91222549efee85421bbf9bf3 
>   3rdparty/libprocess/src/subprocess.cpp 6dfb939ec151f724d383870a806ca3fc8fb0d931 
> 
> 
> Diff: https://reviews.apache.org/r/57973/diff/2/
> 
> 
> Testing
> -------
> 
> Testing done at end of chain.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>