You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2017/07/31 12:59:00 UTC
Re: Review Request 60057: Made explicit that we dispatch to a process
manager.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60057/
-----------------------------------------------------------
(Updated July 31, 2017, 2:58 p.m.)
Review request for mesos and Michael Park.
Changes
-------
Rebased.
Repository: mesos
Description
-------
This member function of 'ProcessManager' was capturing a 'this'
pointer when dispatching to itself, but did not properly use 'defer'
or 'dispatch'. While this pattern is usually suspect, it was safe here
as we can be sure that the process manager lives long enough to safely
invoke the created callback.
This patch removes the capture of 'this' and instead explicitly
references the 'static process_manager' in the created callback to
signal that we rely on external invariants.
Diffs (updated)
-----
3rdparty/libprocess/src/process.cpp 44c3531538691dd9600b9169a83f4fd504f4f089
Diff: https://reviews.apache.org/r/60057/diff/2/
Changes: https://reviews.apache.org/r/60057/diff/1-2/
Testing
-------
* `make check` (Fedora 25 clang-trunk w/ optimizations)
Thanks,
Benjamin Bannier
Re: Review Request 60057: Made explicit that we dispatch to a process
manager.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60057/#review181823
-----------------------------------------------------------
Ship it!
Ship It!
- Michael Park
On July 31, 2017, 5:58 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60057/
> -----------------------------------------------------------
>
> (Updated July 31, 2017, 5:58 a.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This member function of 'ProcessManager' was capturing a 'this'
> pointer when dispatching to itself, but did not properly use 'defer'
> or 'dispatch'. While this pattern is usually suspect, it was safe here
> as we can be sure that the process manager lives long enough to safely
> invoke the created callback.
>
> This patch removes the capture of 'this' and instead explicitly
> references the 'static process_manager' in the created callback to
> signal that we rely on external invariants.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 44c3531538691dd9600b9169a83f4fd504f4f089
>
>
> Diff: https://reviews.apache.org/r/60057/diff/2/
>
>
> Testing
> -------
>
> * `make check` (Fedora 25 clang-trunk w/ optimizations)
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 60057: Made explicit that we dispatch to a process
manager.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60057/#review185877
-----------------------------------------------------------
PASS: Mesos patch 60057 was successfully built and tested.
Reviews applied: `['60057']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60057
- Mesos Reviewbot Windows
On Sept. 21, 2017, 9:26 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60057/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2017, 9:26 a.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This member function of 'ProcessManager' was capturing a 'this'
> pointer when dispatching to itself, but did not properly use 'defer'
> or 'dispatch'. While this pattern is usually suspect, it was safe here
> as we can be sure that the process manager lives long enough to safely
> invoke the created callback.
>
> This patch removes the capture of 'this' and instead explicitly
> references the 'static process_manager' in the created callback to
> signal that we rely on external invariants.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 7aba8f0117d8a336f39f26a82524aae174306d7f
>
>
> Diff: https://reviews.apache.org/r/60057/diff/3/
>
>
> Testing
> -------
>
> * `make check` (Fedora 25 clang-trunk w/ optimizations)
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 60057: Made explicit that we dispatch to a process
manager.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60057/#review185908
-----------------------------------------------------------
Patch looks great!
Reviews applied: [60057]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Sept. 21, 2017, 5:26 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60057/
> -----------------------------------------------------------
>
> (Updated Sept. 21, 2017, 5:26 a.m.)
>
>
> Review request for mesos and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This member function of 'ProcessManager' was capturing a 'this'
> pointer when dispatching to itself, but did not properly use 'defer'
> or 'dispatch'. While this pattern is usually suspect, it was safe here
> as we can be sure that the process manager lives long enough to safely
> invoke the created callback.
>
> This patch removes the capture of 'this' and instead explicitly
> references the 'static process_manager' in the created callback to
> signal that we rely on external invariants.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/process.cpp 7aba8f0117d8a336f39f26a82524aae174306d7f
>
>
> Diff: https://reviews.apache.org/r/60057/diff/3/
>
>
> Testing
> -------
>
> * `make check` (Fedora 25 clang-trunk w/ optimizations)
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 60057: Made explicit that we dispatch to a process
manager.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60057/
-----------------------------------------------------------
(Updated Sept. 21, 2017, 11:26 a.m.)
Review request for mesos and Michael Park.
Changes
-------
Rebased.
Repository: mesos
Description
-------
This member function of 'ProcessManager' was capturing a 'this'
pointer when dispatching to itself, but did not properly use 'defer'
or 'dispatch'. While this pattern is usually suspect, it was safe here
as we can be sure that the process manager lives long enough to safely
invoke the created callback.
This patch removes the capture of 'this' and instead explicitly
references the 'static process_manager' in the created callback to
signal that we rely on external invariants.
Diffs (updated)
-----
3rdparty/libprocess/src/process.cpp 7aba8f0117d8a336f39f26a82524aae174306d7f
Diff: https://reviews.apache.org/r/60057/diff/3/
Changes: https://reviews.apache.org/r/60057/diff/2-3/
Testing
-------
* `make check` (Fedora 25 clang-trunk w/ optimizations)
Thanks,
Benjamin Bannier