Fix getSnapshot to work on ObserverManager thread

Summary: It's possible for user code to be run in the ObserverManager thread outside of the refresh call (e.g. from Core destructor), so we can't rely on inManagerThread check to guarantee that Dependency recorder is active.

Reviewed By: yfeldblum

Differential Revision: D17805252

fbshipit-source-id: 5a858be0cf204da1342dc182e25797fd130c0458

folly/experimental/observer/detail/Core.cpp

@@ -23,7 +23,7 @@ namespace folly {
 namespace observer_detail {
 
 Core::VersionedData Core::getData() {
-  if (!ObserverManager::inManagerThread()) {
+  if (!ObserverManager::DependencyRecorder::isActive()) {
     return data_.copy();
   }
 

folly/experimental/observer/detail/ObserverManager.h

@@ -151,6 +151,10 @@ class ObserverManager {
       currentDependencies_ = &dependencies_;
     }
 
+    static bool isActive() {
+      return currentDependencies_;
+    }
+
     static void markDependency(Core::Ptr dependency) {
       DCHECK(inManagerThread());
       DCHECK(currentDependencies_);

folly/experimental/observer/test/ObserverTest.cpp

@@ -431,3 +431,46 @@ TEST(Observer, IgnoreUpdates) {
   folly::observer_detail::ObserverManager::waitForAllUpdates();
   EXPECT_EQ(3, callbackCalled);
 }
+
+TEST(Observer, GetSnapshotOnManagerThread) {
+  auto observer42 = folly::observer::makeObserver([] { return 42; });
+
+  folly::observer::SimpleObservable<int> observable(1);
+
+  folly::Baton<> startBaton;
+  folly::Baton<> finishBaton;
+  folly::Baton<> destructorBaton;
+
+  {
+    finishBaton.post();
+    auto slowObserver = folly::observer::makeObserver(
+        [guard = folly::makeGuard([observer42, &destructorBaton]() {
+           // We expect this to be called on a ObserverManager thread, but
+           // outside of processing an observer updates.
+           observer42.getSnapshot();
+           destructorBaton.post();
+         }),
+         observer = observable.getObserver(),
+         &startBaton,
+         &finishBaton] {
+          startBaton.post();
+          finishBaton.wait();
+          finishBaton.reset();
+          return **observer;
+        });
+
+    EXPECT_EQ(1, **slowObserver);
+
+    startBaton.reset();
+    finishBaton.post();
+    observable.setValue(2);
+    folly::observer_detail::ObserverManager::waitForAllUpdates();
+    EXPECT_EQ(2, **slowObserver);
+
+    startBaton.reset();
+    observable.setValue(3);
+    startBaton.wait();
+  }
+  finishBaton.post();
+  destructorBaton.wait();
+}