cleaning up RequestContext

Test Plan: built folly

Reviewed By: mshneer@fb.com

Subscribers: davejwatson, mshneer, trunkagent, alandau, bmatheny, folly-diffs@, yfeldblum, chalfant

FB internal diff: D2096146

Tasks: 6337419

Signature: t1:2096146:1432658670:4ff00e4274412519dc3b02d2a849b7ff289ac979

diff --git a/folly/Singleton.cpp b/folly/Singleton.cpp
index d389f6efaf57d5b11b252a8f854ee1563a1a359e..a9ca67f6d2d2f676c643b7a1f68bfd4724357692 100644 (file)
--- a/folly/Singleton.cpp
+++ b/folly/Singleton.cpp
@@ -102,7 +102,7 @@ void SingletonVault::reenableInstances() {
 }
 
 void SingletonVault::scheduleDestroyInstances() {
-  RequestContext::getStaticContext();
+  RequestContext::saveContext();
 
   class SingletonVaultDestructor {
    public:

diff --git a/folly/Singleton.h b/folly/Singleton.h
index 4a33205c0ff87ad0609be5522fa8b6525dee7223..8eff26f22a8b923aace0e14a9375b980a042582d 100644 (file)
--- a/folly/Singleton.h
+++ b/folly/Singleton.h
@@ -306,7 +306,7 @@ class SingletonVault {
   // Mark registration is complete; no more singletons can be
   // registered at this point.
   void registrationComplete() {
-    RequestContext::getStaticContext();
+    RequestContext::saveContext();
     std::atexit([](){ SingletonVault::singleton()->destroyInstances(); });
 
     RWSpinLock::WriteHolder wh(&stateMutex_);

diff --git a/folly/io/async/AsyncTimeout.cpp b/folly/io/async/AsyncTimeout.cpp
index 0b371d776f21fdfc9452d13328afaf41c1fdc4d7..31a1c0b5aadb803f2ecdc84e763291d6f42ad75c 100644 (file)
--- a/folly/io/async/AsyncTimeout.cpp
+++ b/folly/io/async/AsyncTimeout.cpp
@@ -36,7 +36,7 @@ AsyncTimeout::AsyncTimeout(TimeoutManager* timeoutManager)
   timeoutManager_->attachTimeoutManager(
       this,
       TimeoutManager::InternalEnum::NORMAL);
-  RequestContext::getStaticContext();
+  RequestContext::saveContext();
 }
 
 AsyncTimeout::AsyncTimeout(EventBase* eventBase)
@@ -49,7 +49,7 @@ AsyncTimeout::AsyncTimeout(EventBase* eventBase)
       this,
       TimeoutManager::InternalEnum::NORMAL);
   }
-  RequestContext::getStaticContext();
+  RequestContext::saveContext();
 }
 
 AsyncTimeout::AsyncTimeout(TimeoutManager* timeoutManager,
@@ -59,7 +59,7 @@ AsyncTimeout::AsyncTimeout(TimeoutManager* timeoutManager,
   event_set(&event_, -1, EV_TIMEOUT, &AsyncTimeout::libeventCallback, this);
   event_.ev_base = nullptr;
   timeoutManager_->attachTimeoutManager(this, internal);
-  RequestContext::getStaticContext();
+  RequestContext::saveContext();
 }
 
 AsyncTimeout::AsyncTimeout(EventBase* eventBase, InternalEnum internal)
@@ -68,13 +68,13 @@ AsyncTimeout::AsyncTimeout(EventBase* eventBase, InternalEnum internal)
   event_set(&event_, -1, EV_TIMEOUT, &AsyncTimeout::libeventCallback, this);
   event_.ev_base = nullptr;
   timeoutManager_->attachTimeoutManager(this, internal);
-  RequestContext::getStaticContext();
+  RequestContext::saveContext();
 }
 
 AsyncTimeout::AsyncTimeout(): timeoutManager_(nullptr) {
   event_set(&event_, -1, EV_TIMEOUT, &AsyncTimeout::libeventCallback, this);
   event_.ev_base = nullptr;
-  RequestContext::getStaticContext();
+  RequestContext::saveContext();
 }
 
 AsyncTimeout::~AsyncTimeout() {

diff --git a/folly/io/async/EventBase.cpp b/folly/io/async/EventBase.cpp
index daf4049b81efa9091a76a73f5821f652b0f3d336..73a6af57af346823a3b6a0c8b207d4b25a7ee173 100644 (file)
--- a/folly/io/async/EventBase.cpp
+++ b/folly/io/async/EventBase.cpp
@@ -174,7 +174,7 @@ EventBase::EventBase(bool enableTimeMeasurement)
   }
   VLOG(5) << "EventBase(): Created.";
   initNotificationQueue();
-  RequestContext::getStaticContext();
+  RequestContext::saveContext();
 }
 
 // takes ownership of the event_base
@@ -199,7 +199,7 @@ EventBase::EventBase(event_base* evb, bool enableTimeMeasurement)
     throw std::invalid_argument("EventBase(): event base cannot be nullptr");
   }
   initNotificationQueue();
-  RequestContext::getStaticContext();
+  RequestContext::saveContext();
 }
 
 EventBase::~EventBase() {

diff --git a/folly/io/async/NotificationQueue.h b/folly/io/async/NotificationQueue.h
index cbbed8c417eae4daf55f56fdcd00e528c28bace5..6bdf16fae361fdca48467fb2e0306ffdeaa35e28 100644 (file)
--- a/folly/io/async/NotificationQueue.h
+++ b/folly/io/async/NotificationQueue.h
@@ -228,7 +228,7 @@ class NotificationQueue {
       pid_(getpid()),
       queue_() {
 
-    RequestContext::getStaticContext();
+    RequestContext::saveContext();
 
 #ifdef FOLLY_HAVE_EVENTFD
     if (fdType == FdType::EVENTFD) {

diff --git a/folly/io/async/Request.cpp b/folly/io/async/Request.cpp
deleted file mode 100644 (file)
index 5011513..0000000
--- a/folly/io/async/Request.cpp
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
- * Copyright 2015 Facebook, Inc.
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-#include <folly/io/async/Request.h>
-
-namespace folly {
-
-RequestContext* defaultContext;
-
-}

diff --git a/folly/io/async/Request.h b/folly/io/async/Request.h
index bd369e1a3643498c8b4a60b7844d21caec88edd8..490c69b08a26c566a87f43175c7ccf8e0a84f9f3 100644 (file)
--- a/folly/io/async/Request.h
+++ b/folly/io/async/Request.h
@@ -41,8 +41,6 @@ class RequestContext;
 // If you do not call create() to create a unique request context,
 // this default request context will always be returned, and is never
 // copied between threads.
-extern RequestContext* defaultContext;
-
 class RequestContext {
  public:
   // Create a unique requext context for this request.
@@ -54,13 +52,12 @@ class RequestContext {
 
   // Get the current context.
   static RequestContext* get() {
-    if (getStaticContext() == nullptr) {
-      if (defaultContext == nullptr) {
-        defaultContext = new RequestContext;
-      }
-      return defaultContext;
+    auto context = getStaticContext();
+    if (!context) {
+      static RequestContext defaultContext;
+      return std::addressof(defaultContext);
     }
-    return getStaticContext().get();
+    return context.get();
   }
 
   // The following API may be used to set per-request data in a thread-safe way.
@@ -108,31 +105,27 @@ class RequestContext {
 
   static std::shared_ptr<RequestContext>
   setContext(std::shared_ptr<RequestContext> ctx) {
-    std::shared_ptr<RequestContext> old_ctx;
-    if (getStaticContext()) {
-      old_ctx = getStaticContext();
-    }
-    getStaticContext() = ctx;
-    return old_ctx;
+    using std::swap;
+    swap(ctx, getStaticContext());
+    return ctx;
   }
 
   static std::shared_ptr<RequestContext> saveContext() {
     return getStaticContext();
   }
 
+ private:
   // Used to solve static destruction ordering issue.  Any static object
   // that uses RequestContext must call this function in its constructor.
   //
   // See below link for more details.
   // http://stackoverflow.com/questions/335369/
   // finding-c-static-initialization-order-problems#335746
-  static std::shared_ptr<RequestContext>&
-  getStaticContext() {
+  static std::shared_ptr<RequestContext> &getStaticContext() {
     static folly::ThreadLocal<std::shared_ptr<RequestContext> > context;
     return *context;
   }
 
- private:
   folly::RWSpinLock lock;
   std::map<std::string, std::unique_ptr<RequestData>> data_;
 };