Commit 0cb5aa0f authored by Pranjal Raihan's avatar Pranjal Raihan Committed by Facebook GitHub Bot

Expose folly::observer::waitForAllUpdates as public API

Summary:
In most cases, `folly::observer::waitForAllUpdates` should not be needed. However, the asynchronous nature of observers break some basic assumptions that most users expect. Such as:
```
thing.set(value);
EXPECT_EQ(thing.get(), value);
```
If `thing` is using `folly::observer` underneath the hood, the asynchrony is passed on to the user which is very easy to overlook, leading to bugs. The API of `thing` might want to enforce that when `thing.set` returns, all observed values will have been updated.
Blocking is a perfectly okay solution in cases we don't expect an observer to be updated frequently.

Differential Revision: D25542376

fbshipit-source-id: c83909807fbf36e0311a45b667da94c43751a1da
parent e8aa07ff
......@@ -162,5 +162,9 @@ ObserverCreator<Observable, Traits>::getObserver() && {
return observer;
}
inline void waitForAllUpdates() {
observer_detail::ObserverManager::waitForAllUpdates();
}
} // namespace observer
} // namespace folly
......@@ -366,6 +366,26 @@ auto makeAtomicObserver(F&& creator) {
return makeAtomicObserver(makeObserver(std::forward<F>(creator)));
}
/**
* Blocks the current thread until all Observer updates have propagated through
* observer dependency chains. Since Observers are updated asynchronously on a
* background thread, updated values may not be visible immediately after the
* value is set. i.e. the following code does not work as expected.
*
* SimpleObservable<int> observable{0};
* auto observer = observable.getObserver();
* observable.setValue(42);
* EXPECT_EQ(**observer, 42); // fails
*
* If you want to read the observer after setting a value, you need to block
* until the update is propagated. Therefore, you should only call this function
* sparingly and definitely NOT in hot paths.
*
* If possible, prefer creating a dependent observer using
* `folly::observer::makeObserver` rather than blocking.
*/
inline void waitForAllUpdates();
template <typename T, bool CacheInThreadLocal>
struct ObserverTraits {};
......
......@@ -266,7 +266,7 @@ TEST(Observer, StressMultipleUpdates) {
for (size_t i = 1; i <= numIters; ++i) {
observable1.setValue(i);
observable2.setValue(i);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(i * i, **observer);
}
}
......@@ -305,7 +305,7 @@ TEST(ReadMostlyTLObserver, Update) {
observable.setValue(24);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(*readMostlyObserver.getShared(), 24);
}
......@@ -352,7 +352,7 @@ TEST(Observer, SubscribeCallback) {
EXPECT_EQ(3, getCallsStart);
EXPECT_EQ(3, getCallsFinish);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
slowGet = true;
cobThread = std::thread([] { updatesCob(); });
......@@ -432,11 +432,11 @@ TEST(Observer, WaitForAllUpdates) {
EXPECT_EQ(42, **observer);
observable.setValue(43);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(43, **observer);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
}
TEST(Observer, IgnoreUpdates) {
......@@ -455,15 +455,15 @@ TEST(Observer, IgnoreUpdates) {
EXPECT_EQ(1, callbackCalled);
observable.setValue(43);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(2, callbackCalled);
observable.setValue(45);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(2, callbackCalled);
observable.setValue(46);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(3, callbackCalled);
}
......@@ -499,7 +499,7 @@ TEST(Observer, GetSnapshotOnManagerThread) {
startBaton.reset();
finishBaton.post();
observable.setValue(2);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(2, **slowObserver);
startBaton.reset();
......@@ -544,19 +544,19 @@ TEST(Observer, MakeValueObserver) {
.addCallback([&](auto snapshot) {
observedValues2.push_back(snapshot->value_);
});
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
observable.setValue(ValueStruct(1, 2));
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
observable.setValue(ValueStruct(2, 3));
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
observable.setValue(ValueStruct(2, 4));
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
observable.setValue(ValueStruct(3, 5));
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(observedIds, std::vector<int>({1, 2, 3, 4, 5}));
EXPECT_EQ(observedValues, std::vector<int>({1, 2, 3}));
......@@ -588,7 +588,7 @@ TEST(Observer, AtomicObserver) {
EXPECT_EQ(*observer, 42);
EXPECT_EQ(*observerCopy, 42);
observable.setValue(24);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(*observer, 24);
EXPECT_EQ(*observerCopy, 24);
......@@ -596,7 +596,7 @@ TEST(Observer, AtomicObserver) {
EXPECT_EQ(*observer, 12);
EXPECT_EQ(*observerCopy, 24);
observable2.setValue(15);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(*observer, 15);
EXPECT_EQ(*observerCopy, 24);
......@@ -607,7 +607,7 @@ TEST(Observer, AtomicObserver) {
makeAtomicObserver([o = observer] { return *o + 1; });
EXPECT_EQ(*dependentObserver, 16);
observable2.setValue(20);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(*dependentObserver, 21);
}
......@@ -628,18 +628,18 @@ TEST(Observer, Unwrap) {
EXPECT_EQ(**observer, 1);
selectorObservable.setValue(false);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(**observer, 2);
falseObservable.setValue(3);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(**observer, 3);
trueObservable.setValue(4);
selectorObservable.setValue(true);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(**observer, 4);
}
......@@ -652,17 +652,17 @@ TEST(Observer, UnwrapSimpleObservable) {
EXPECT_EQ(1, **o);
a.setValue(3);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(3, **o);
observable.setValue(b.getObserver());
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(2, **o);
b.setValue(4);
folly::observer_detail::ObserverManager::waitForAllUpdates();
folly::observer::waitForAllUpdates();
EXPECT_EQ(4, **o);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment