You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by be...@apache.org on 2016/12/16 16:27:53 UTC

mesos git commit: Avoided capturing `this` in `process::loop`.

Repository: mesos
Updated Branches:
  refs/heads/master ead829bff -> 0d612c0bf


Avoided capturing `this` in `process::loop`.

The way process' `Loop` abstraction is used ensures that `Loop`
instances life long enough that outstanding continuations are not
working with destructed data. This is done by capturing
lifetime-extending handles by value into any continuation.

Nevertheless, even though continuations always have access to save
handles (captured via `self`), the code was still accessing `Loop`
members via explicitly captured `this` pointers. This appears
needlessly alarming since safe access would always be possible via
`self`.

This change replaces access to `Loop` data via potentially (but
currently not actually) unsafe `this` pointers with access via the
safe `self`.

Review: https://reviews.apache.org/r/54796/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0d612c0b
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0d612c0b
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0d612c0b

Branch: refs/heads/master
Commit: 0d612c0bfc1c976be56eee477cf04af942696201
Parents: ead829b
Author: Benjamin Bannier <be...@mesosphere.io>
Authored: Fri Dec 16 08:27:41 2016 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Dec 16 08:27:41 2016 -0800

----------------------------------------------------------------------
 3rdparty/libprocess/include/process/loop.hpp | 30 +++++++++++------------
 1 file changed, 15 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0d612c0b/3rdparty/libprocess/include/process/loop.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/loop.hpp b/3rdparty/libprocess/include/process/loop.hpp
index ac54b63..b35f7e6 100644
--- a/3rdparty/libprocess/include/process/loop.hpp
+++ b/3rdparty/libprocess/include/process/loop.hpp
@@ -161,7 +161,7 @@ public:
     // something like `atomic_load` and `atomic_store` with
     // `shared_ptr` so that we can swap the current future(s)
     // atomically.
-    promise.future().onDiscard([weak_self, this]() {
+    promise.future().onDiscard([weak_self]() {
       auto self = weak_self.lock();
       if (self) {
         // We need to make a copy of the current `discard` function so
@@ -170,8 +170,8 @@ public:
         // callbacks that we have added in `run` to execute which may
         // deadlock attempting to re-acquire `mutex`!
         std::function<void()> f = []() {};
-        synchronized (mutex) {
-          f = discard;
+        synchronized (self->mutex) {
+          f = self->discard;
         }
         f();
       }
@@ -179,8 +179,8 @@ public:
 
     if (pid.isSome()) {
       // Start the loop using `pid` as the execution context.
-      dispatch(pid.get(), [self, this]() {
-        run(iterate());
+      dispatch(pid.get(), [self]() {
+        self->run(self->iterate());
       });
     } else {
       run(iterate());
@@ -210,17 +210,17 @@ public:
           return;
         }
       } else {
-        auto continuation = [self, this](const Future<bool>& condition) {
+        auto continuation = [self](const Future<bool>& condition) {
           if (condition.isReady()) {
             if (condition.get()) {
-              run(iterate());
+              self->run(self->iterate());
             } else {
-              promise.set(Nothing());
+              self->promise.set(Nothing());
             }
           } else if (condition.isFailed()) {
-            promise.fail(condition.failure());
+            self->promise.fail(condition.failure());
           } else if (condition.isDiscarded()) {
-            promise.discard();
+            self->promise.discard();
           }
         };
 
@@ -232,7 +232,7 @@ public:
 
         if (!promise.future().hasDiscard()) {
           synchronized (mutex) {
-            discard = [=]() mutable { condition.discard(); };
+            self->discard = [=]() mutable { condition.discard(); };
           }
         }
 
@@ -249,13 +249,13 @@ public:
       }
     }
 
-    auto continuation = [self, this](const Future<T>& next) {
+    auto continuation = [self](const Future<T>& next) {
       if (next.isReady()) {
-        run(next);
+        self->run(next);
       } else if (next.isFailed()) {
-        promise.fail(next.failure());
+        self->promise.fail(next.failure());
       } else if (next.isDiscarded()) {
-        promise.discard();
+        self->promise.discard();
       }
     };