You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ch...@apache.org on 2019/04/11 16:05:45 UTC

[mesos] 01/02: Fixed potential use-after-free bug in storage local resource provider.

This is an automated email from the ASF dual-hosted git repository.

chhsiao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit e3504a6a86558d264a939d974fdb5c257ab53f5d
Author: Benjamin Bannier <be...@mesosphere.io>
AuthorDate: Thu Apr 11 08:55:43 2019 -0700

    Fixed potential use-after-free bug in storage local resource provider.
    
    The storage local resource provider manages a metrics instance which is
    shared with the service and volume managers it also holds; these
    services do not manage the lifetime of the metrics instance.
    
    This patch fixes the lifetime of the metrics instance.
    
    Review: https://reviews.apache.org/r/70454/
---
 src/resource_provider/storage/provider.cpp | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/resource_provider/storage/provider.cpp b/src/resource_provider/storage/provider.cpp
index b2ca5d0..999fe95 100644
--- a/src/resource_provider/storage/provider.cpp
+++ b/src/resource_provider/storage/provider.cpp
@@ -377,6 +377,19 @@ private:
 
   Runtime runtime;
 
+  // NOTE: `metrics` must be destructed after `volumeManager` and
+  // `serviceManager` since they hold a pointer to it.
+  struct Metrics : public csi::Metrics
+  {
+    explicit Metrics(const string& prefix);
+    ~Metrics();
+
+    hashmap<Offer::Operation::Type, PushGauge> operations_pending;
+    hashmap<Offer::Operation::Type, Counter> operations_finished;
+    hashmap<Offer::Operation::Type, Counter> operations_failed;
+    hashmap<Offer::Operation::Type, Counter> operations_dropped;
+  } metrics;
+
   // NOTE: `serviceManager` must be destructed after `volumeManager` since the
   // latter holds a pointer of the former.
   Owned<ServiceManager> serviceManager;
@@ -401,17 +414,6 @@ private:
   // keeps track of pending operations that disallow reconciliation, and ensures
   // that any reconciliation waits for these operations to finish.
   Sequence sequence;
-
-  struct Metrics : public csi::Metrics
-  {
-    explicit Metrics(const string& prefix);
-    ~Metrics();
-
-    hashmap<Offer::Operation::Type, PushGauge> operations_pending;
-    hashmap<Offer::Operation::Type, Counter> operations_finished;
-    hashmap<Offer::Operation::Type, Counter> operations_failed;
-    hashmap<Offer::Operation::Type, Counter> operations_dropped;
-  } metrics;
 };
 
 
@@ -434,9 +436,9 @@ StorageLocalResourceProviderProcess::StorageLocalResourceProviderProcess(
     slaveId(_slaveId),
     authToken(_authToken),
     strict(_strict),
+    metrics("resource_providers/" + info.type() + "." + info.name() + "/"),
     resourceVersion(id::UUID::random()),
-    sequence("storage-local-resource-provider-sequence"),
-    metrics("resource_providers/" + info.type() + "." + info.name() + "/")
+    sequence("storage-local-resource-provider-sequence")
 {
   diskProfileAdaptor = DiskProfileAdaptor::getAdaptor();
   CHECK_NOTNULL(diskProfileAdaptor.get());