X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=blobdiff_plain;f=folly%2FThreadCachedInt.h;h=9b7c3136f117518e6e71fdfd88249880b9e9192b;hp=698e3bbca8e53f1e77eb484814282046c28a2125;hb=fd0b3090840d742492793fe306185550f0300283;hpb=52fceaf485e63d459d2df5858ffdbeaafe851dbf

diff --git a/folly/ThreadCachedInt.h b/folly/ThreadCachedInt.h
index 698e3bbc..9b7c3136 100644
--- a/folly/ThreadCachedInt.h
+++ b/folly/ThreadCachedInt.h
@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 Facebook, Inc.
+ * Copyright 2017 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,15 +20,14 @@
  * @author Spencer Ahrens (sahrens)
  */
 
-#ifndef FOLLY_THREADCACHEDINT_H
-#define FOLLY_THREADCACHEDINT_H
+#pragma once
 
 #include <atomic>
 
 #include <boost/noncopyable.hpp>
 
-#include "folly/Likely.h"
-#include "folly/ThreadLocal.h"
+#include <folly/Likely.h>
+#include <folly/ThreadLocal.h>
 
 namespace folly {
 
@@ -48,7 +47,7 @@ class ThreadCachedInt : boost::noncopyable {
 
   void increment(IntT inc) {
     auto cache = cache_.get();
-    if (UNLIKELY(cache == nullptr || cache->parent_ == nullptr)) {
+    if (UNLIKELY(cache == nullptr)) {
       cache = new IntCache(*this);
       cache_.reset(cache);
     }
@@ -64,8 +63,11 @@ class ThreadCachedInt : boost::noncopyable {
   // Reads the current value plus all the cached increments.  Requires grabbing
   // a lock, so this is significantly slower than readFast().
   IntT readFull() const {
+    // This could race with thread destruction and so the access lock should be
+    // acquired before reading the current value
+    auto accessor = cache_.accessAllThreads();
     IntT ret = readFast();
-    for (const auto& cache : cache_.accessAllThreads()) {
+    for (const auto& cache : accessor) {
       if (!cache.reset_.load(std::memory_order_acquire)) {
         ret += cache.val_.load(std::memory_order_relaxed);
       }
@@ -83,8 +85,11 @@ class ThreadCachedInt : boost::noncopyable {
   // little off, however, but it should be much better than calling readFull()
   // and set(0) sequentially.
   IntT readFullAndReset() {
+    // This could race with thread destruction and so the access lock should be
+    // acquired before reading the current value
+    auto accessor = cache_.accessAllThreads();
     IntT ret = readFastAndReset();
-    for (auto& cache : cache_.accessAllThreads()) {
+    for (auto& cache : accessor) {
       if (!cache.reset_.load(std::memory_order_acquire)) {
         ret += cache.val_.load(std::memory_order_relaxed);
         cache.reset_.store(true, std::memory_order_release);
@@ -105,7 +110,10 @@ class ThreadCachedInt : boost::noncopyable {
   ThreadCachedInt& operator-=(IntT inc) { increment(-inc); return *this; }
   // pre-increment (we don't support post-increment)
   ThreadCachedInt& operator++() { increment(1); return *this; }
-  ThreadCachedInt& operator--() { increment(-1); return *this; }
+  ThreadCachedInt& operator--() {
+    increment(IntT(-1));
+    return *this;
+  }
 
   // Thread-safe set function.
   // This is a best effort implementation. In some edge cases, there could be
@@ -117,19 +125,11 @@ class ThreadCachedInt : boost::noncopyable {
     target_.store(newVal, std::memory_order_release);
   }
 
-  // This is a little tricky - it's possible that our IntCaches are still alive
-  // in another thread and will get destroyed after this destructor runs, so we
-  // need to make sure we signal that this parent is dead.
-  ~ThreadCachedInt() {
-    for (auto& cache : cache_.accessAllThreads()) {
-      cache.parent_ = nullptr;
-    }
-  }
-
  private:
   std::atomic<IntT> target_;
   std::atomic<uint32_t> cacheSize_;
-  ThreadLocalPtr<IntCache,Tag> cache_; // Must be last for dtor ordering
+  ThreadLocalPtr<IntCache, Tag, AccessModeStrict>
+      cache_; // Must be last for dtor ordering
 
   // This should only ever be modified by one thread
   struct IntCache {
@@ -167,13 +167,9 @@ class ThreadCachedInt : boost::noncopyable {
     }
 
     ~IntCache() {
-      if (parent_) {
-        flush();
-      }
+      flush();
     }
   };
 };
 
-}
-
-#endif
+} // namespace folly