mesos git commit: Replaced volatile, GCC intrinsics with std::atomic.

[jo...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master 6da93b559 -> 474a43e61


Replaced volatile, GCC intrinsics with std::atomic.

See MESOS-3326. We adopted std::atomic in most of the code base earlier
(commits 87de003c6e8a, 4b938052b6af, and 4a01850c5540), but a few places
were omitted; those locations are fixed by this commit.

There's one last place to fix: we use the GCC intrinsic
__sync_synchronize() in 3rdparty/libprocess/include/process/logging.h.
Because that is used to protect modifications to the FLAGS_v variable
defined by glog, we can't easily adapt it to use std::atomic.

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


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

Branch: refs/heads/master
Commit: 474a43e612b693e65e8f20d87a4261accd4679e5
Parents: 6da93b5
Author: Neil Conway <ne...@gmail.com>
Authored: Tue Oct 20 16:33:29 2015 -0400
Committer: Joris Van Remoortere <jo...@gmail.com>
Committed: Tue Oct 20 16:36:46 2015 -0400

----------------------------------------------------------------------
 3rdparty/libprocess/include/process/owned.hpp   | 19 +++---
 3rdparty/libprocess/include/process/shared.hpp  |  8 ++-
 3rdparty/libprocess/src/tests/process_tests.cpp | 71 ++++++++++----------
 3 files changed, 50 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/474a43e6/3rdparty/libprocess/include/process/owned.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/owned.hpp b/3rdparty/libprocess/include/process/owned.hpp
index 17c4261..1b41477 100644
--- a/3rdparty/libprocess/include/process/owned.hpp
+++ b/3rdparty/libprocess/include/process/owned.hpp
@@ -15,6 +15,7 @@
 #ifndef __PROCESS_OWNED_HPP__
 #define __PROCESS_OWNED_HPP__
 
+#include <atomic>
 #include <memory>
 
 #include <glog/logging.h>
@@ -64,7 +65,7 @@ private:
     explicit Data(T* t);
     ~Data();
 
-    T* volatile t; // The pointer 't' is volatile.
+    std::atomic<T*> t;
   };
 
   std::shared_ptr<Data> data;
@@ -159,14 +160,14 @@ Shared<T> Owned<T>::share()
   }
 
   // Atomically set the pointer 'data->t' to NULL.
-  T* t = __sync_fetch_and_and(&data->t, NULL);
-  if (t == NULL) {
+  T* old = data->t.exchange(NULL);
+  if (old == NULL) {
     // The ownership of this pointer has already been lost.
     return Shared<T>(NULL);
   }
 
   data.reset();
-  return Shared<T>(t);
+  return Shared<T>(old);
 }
 
 
@@ -179,14 +180,14 @@ T* Owned<T>::release()
   }
 
   // Atomically set the pointer 'data->t' to NULL.
-  T* t = __sync_fetch_and_and(&data->t, NULL);
-  if (t == NULL) {
+  T* old = data->t.exchange(NULL);
+  if (old == NULL) {
     // The ownership of this pointer has already been lost.
     return NULL;
   }
 
   data.reset();
-  return t;
+  return old;
 }
 
 
@@ -198,9 +199,7 @@ Owned<T>::Data::Data(T* _t)
 template <typename T>
 Owned<T>::Data::~Data()
 {
-  if (t != NULL) {
-    delete t;
-  }
+  delete t.load();
 }
 
 } // namespace process {

http://git-wip-us.apache.org/repos/asf/mesos/blob/474a43e6/3rdparty/libprocess/include/process/shared.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/shared.hpp b/3rdparty/libprocess/include/process/shared.hpp
index 021807b..4f602cc 100644
--- a/3rdparty/libprocess/include/process/shared.hpp
+++ b/3rdparty/libprocess/include/process/shared.hpp
@@ -15,6 +15,7 @@
 #ifndef __PROCESS_SHARED_HPP__
 #define __PROCESS_SHARED_HPP__
 
+#include <atomic>
 #include <memory>
 
 #include <glog/logging.h>
@@ -64,7 +65,7 @@ private:
     ~Data();
 
     T* t;
-    volatile bool owned;
+    std::atomic_bool owned;
     Promise<Owned<T>> promise;
   };
 
@@ -167,7 +168,8 @@ Future<Owned<T>> Shared<T>::own()
     return Owned<T>(NULL);
   }
 
-  if (!__sync_bool_compare_and_swap(&data->owned, false, true)) {
+  bool false_value = false;
+  if (!data->owned.compare_exchange_strong(false_value, true)) {
     return Failure("Ownership has already been transferred");
   }
 
@@ -185,7 +187,7 @@ Shared<T>::Data::Data(T* _t)
 template <typename T>
 Shared<T>::Data::~Data()
 {
-  if (owned) {
+  if (owned.load()) {
     promise.set(Owned<T>(t));
   } else {
     delete t;

http://git-wip-us.apache.org/repos/asf/mesos/blob/474a43e6/3rdparty/libprocess/src/tests/process_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/process_tests.cpp b/3rdparty/libprocess/src/tests/process_tests.cpp
index a3c57a2..708b8d9 100644
--- a/3rdparty/libprocess/src/tests/process_tests.cpp
+++ b/3rdparty/libprocess/src/tests/process_tests.cpp
@@ -21,6 +21,7 @@
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 
+#include <atomic>
 #include <sstream>
 #include <string>
 #include <tuple>
@@ -253,10 +254,10 @@ TEST(ProcessTest, Repair)
 
 
 
-Future<Nothing> after(volatile bool* executed, const Future<Nothing>& future)
+Future<Nothing> after(std::atomic_bool* executed, const Future<Nothing>& future)
 {
   EXPECT_TRUE(future.hasDiscard());
-  *executed = true;
+  executed->store(true);
   return Failure("Failure");
 }
 
@@ -267,7 +268,7 @@ TEST(ProcessTest, After1)
 {
   Clock::pause();
 
-  volatile bool executed = false;
+  std::atomic_bool executed(false);
 
   Future<Nothing> future = Future<Nothing>()
     .after(Hours(42), lambda::bind(&after, &executed, lambda::_1));
@@ -291,7 +292,7 @@ TEST(ProcessTest, After1)
 
   AWAIT_FAILED(future);
 
-  EXPECT_TRUE(executed);
+  EXPECT_TRUE(executed.load());
 
   Clock::resume();
 }
@@ -303,7 +304,7 @@ TEST(ProcessTest, After2)
 {
   Clock::pause();
 
-  volatile bool executed = false;
+  std::atomic_bool executed(false);
 
   Promise<Nothing> promise;
 
@@ -332,7 +333,7 @@ TEST(ProcessTest, After2)
   // callback to execute.
   Clock::advance(Hours(21));
 
-  EXPECT_FALSE(executed);
+  EXPECT_FALSE(executed.load());
 
   Clock::resume();
 }
@@ -407,9 +408,9 @@ Future<bool> inner1(const Future<bool>& future)
 }
 
 
-Future<int> inner2(volatile bool* executed, const Future<int>& future)
+Future<int> inner2(std::atomic_bool* executed, const Future<int>& future)
 {
-  *executed = true;
+  executed->store(true);
   return future;
 }
 
@@ -421,7 +422,7 @@ TEST(ProcessTest, Discard1)
   Promise<bool> promise1;
   Promise<int> promise2;
 
-  volatile bool executed = false;
+  std::atomic_bool executed(false);
 
   Future<int> future = Future<string>("hello world")
     .then(lambda::bind(&inner1, promise1.future()))
@@ -450,7 +451,7 @@ TEST(ProcessTest, Discard1)
   AWAIT_DISCARDED(future);
 
   // And the final lambda should never have executed.
-  ASSERT_FALSE(executed);
+  ASSERT_FALSE(executed.load());
   ASSERT_TRUE(promise2.future().isPending());
 }
 
@@ -462,7 +463,7 @@ TEST(ProcessTest, Discard2)
   Promise<bool> promise1;
   Promise<int> promise2;
 
-  volatile bool executed = false;
+  std::atomic_bool executed(false);
 
   Future<int> future = Future<string>("hello world")
     .then(lambda::bind(&inner1, promise1.future()))
@@ -493,7 +494,7 @@ TEST(ProcessTest, Discard2)
   AWAIT_DISCARDED(future);
 
   // And the final lambda should never have executed.
-  ASSERT_FALSE(executed);
+  ASSERT_FALSE(executed.load());
   ASSERT_TRUE(promise2.future().isPending());
 }
 
@@ -505,7 +506,7 @@ TEST(ProcessTest, Discard3)
   Promise<bool> promise1;
   Promise<int> promise2;
 
-  volatile bool executed = false;
+  std::atomic_bool executed(false);
 
   Future<int> future = Future<string>("hello world")
     .then(lambda::bind(&inner1, promise1.future()))
@@ -534,7 +535,7 @@ TEST(ProcessTest, Discard3)
   AWAIT_FAILED(future);
 
   // And the final lambda should never have executed.
-  ASSERT_FALSE(executed);
+  ASSERT_FALSE(executed.load());
   ASSERT_TRUE(promise2.future().isPending());
 }
 
@@ -753,21 +754,21 @@ TEST(ProcessTest, Defer3)
 {
   ASSERT_TRUE(GTEST_IS_THREADSAFE);
 
-  volatile bool bool1 = false;
-  volatile bool bool2 = false;
+  std::atomic_bool bool1(false);
+  std::atomic_bool bool2(false);
 
   Deferred<void(bool)> set1 =
-    defer([&bool1](bool b) { bool1 = b; });
+    defer([&bool1](bool b) { bool1.store(b); });
 
   set1(true);
 
   Deferred<void(bool)> set2 =
-    defer([&bool2](bool b) { bool2 = b; });
+    defer([&bool2](bool b) { bool2.store(b); });
 
   set2(true);
 
-  while (!bool1);
-  while (!bool2);
+  while (bool1.load() == false);
+  while (bool2.load() == false);
 }
 
 
@@ -995,7 +996,7 @@ TEST(ProcessTest, Delay)
 
   Clock::pause();
 
-  volatile bool timeoutCalled = false;
+  std::atomic_bool timeoutCalled(false);
 
   TimeoutProcess process;
 
@@ -1008,7 +1009,7 @@ TEST(ProcessTest, Delay)
 
   Clock::advance(Seconds(5));
 
-  while (!timeoutCalled);
+  while (timeoutCalled.load() == false);
 
   terminate(process);
   wait(process);
@@ -1036,7 +1037,7 @@ TEST(ProcessTest, Order)
 
   TimeoutProcess process1;
 
-  volatile bool timeoutCalled = false;
+  std::atomic_bool timeoutCalled(false);
 
   EXPECT_CALL(process1, timeout())
     .WillOnce(Assign(&timeoutCalled, true));
@@ -1056,7 +1057,7 @@ TEST(ProcessTest, Order)
 
   dispatch(process2, &OrderProcess::order, process1.self());
 
-  while (!timeoutCalled);
+  while (timeoutCalled.load() == false);
 
   EXPECT_EQ(now + seconds, Clock::now(&process1));
 
@@ -1114,7 +1115,7 @@ TEST(ProcessTest, Exited)
 
   ExitedProcess process(pid);
 
-  volatile bool exitedCalled = false;
+  std::atomic_bool exitedCalled(false);
 
   EXPECT_CALL(process, exited(pid))
     .WillOnce(Assign(&exitedCalled, true));
@@ -1123,7 +1124,7 @@ TEST(ProcessTest, Exited)
 
   terminate(pid);
 
-  while (!exitedCalled);
+  while (exitedCalled.load() == false);
 
   terminate(process);
   wait(process);
@@ -1138,7 +1139,7 @@ TEST(ProcessTest, InjectExited)
 
   ExitedProcess process(pid);
 
-  volatile bool exitedCalled = false;
+  std::atomic_bool exitedCalled(false);
 
   EXPECT_CALL(process, exited(pid))
     .WillOnce(Assign(&exitedCalled, true));
@@ -1147,7 +1148,7 @@ TEST(ProcessTest, InjectExited)
 
   inject::exited(pid, process.self());
 
-  while (!exitedCalled);
+  while (exitedCalled.load() == false);
 
   terminate(process);
   wait(process);
@@ -1211,10 +1212,10 @@ public:
   void afterDispatch()
   {
     os::sleep(Milliseconds(10));
-    calledDispatch = true;
+    calledDispatch.store(true);
   }
 
-  volatile bool calledDispatch;
+  std::atomic_bool calledDispatch;
 };
 
 
@@ -1226,7 +1227,7 @@ TEST(ProcessTest, Settle)
   SettleProcess process;
   spawn(process);
   Clock::settle();
-  ASSERT_TRUE(process.calledDispatch);
+  ASSERT_TRUE(process.calledDispatch.load());
   terminate(process);
   wait(process);
   Clock::resume();
@@ -1302,8 +1303,8 @@ TEST(ProcessTest, Executor)
 {
   ASSERT_TRUE(GTEST_IS_THREADSAFE);
 
-  volatile bool event1Called = false;
-  volatile bool event2Called = false;
+  std::atomic_bool event1Called(false);
+  std::atomic_bool event2Called(false);
 
   EventReceiver receiver;
 
@@ -1329,8 +1330,8 @@ TEST(ProcessTest, Executor)
 
   event2("event2");
 
-  while (!event1Called);
-  while (!event2Called);
+  while (event1Called.load() == false);
+  while (event2Called.load() == false);
 }