Summary:
Remove the form of Future::then that could take a continuation with no parameters from elsewhere in Future.h as it is deprecated, and pending deletion.

In particular this required splitting the multi forms of the functions which still allow flexible typing at this point in time.

Reviewed By: yfeldblum

Differential Revision: D10206118

fbshipit-source-id: bb2ae241d3bfd5b89cae077e667cb39c4cc62797

Summary: Do not check zeroCopyEnabled_ in AsyncSocket::isZeroCopyMsg to avoid an IOBuf leak when zeroCopyEnabled_ changes from true to false

Reviewed By: yfeldblum

Differential Revision: D10285664

fbshipit-source-id: 5f13426f6140668d029a89786eb9a92466d4b1f4

Summary:
I was reading through about the various approaches to small-string
optimization: https://shaharmike.com/cpp/std-string/.
I noticed that Clang can store up to 22 bytes inline but FBString claims it
can do 23, so I read through the code to figure out where the extra byte came
from.

Specifically, I wasn't sure how it could have space to store the size as well
as ensure the buffer was null-terminated. After playing with the code for a bit
(this was further complicated because running the code under ASAN changes the
behavior, which I didn't realize before I started this exploration), I saw
how we don't store the size, but `maxSmallSize - size`, so that null-termination
works out even when `size==23`.

This updates the docs to hopefully save someone else this same exploration.

(Note: this ignores all push blocking failures!)

Reviewed By: ot

Differential Revision: D10258831

fbshipit-source-id: bfc0dd7ae55518af4173625bd719cfd4778180cc

Summary: [Folly] Fix cmake tests build after {D10244149}, which added `instructions::dispatch`.

Reviewed By: ot

Differential Revision: D10287193

fbshipit-source-id: f555229c8fcebccf61cb5577f5bf3edee9c46163

Summary:
[Folly] DRY template args in `Future::thenImplementation`.

Explicitly passing the template arguments which are also the types of non-template arguments is weird. Either limit the explicit template arguments to those which are not also the types of non-template arguments, or pass everything as non-template arguments.

Reviewed By: andriigrynenko

Differential Revision: D10247910

fbshipit-source-id: 64a56c5ea2072803781143b5247145ec2ee09ae0

Summary: as described in title

Reviewed By: Orvid

Differential Revision: D10279251

fbshipit-source-id: 0ab66033193026ccb0b6a06e57557a822ccb8a70

Summary:
- A couple of tests pragma push/pop to avoid `-Wself-move` warning in
  code that does a move-assignment with itself.
- Use `static_cast<T&&>` instead of `std::move` which will suppress
  the warning still without the need for pragma push/pop.
Pull Request resolved: https://github.com/facebook/folly/pull/946

Reviewed By: Orvid

Differential Revision: D10253159

Pulled By: yfeldblum

fbshipit-source-id: 9d75db95018e6115a195f246dbd93b0a5935d80d

Summary: [Folly] Enable `Fingerprint` test in CMake builds.

Reviewed By: Orvid

Differential Revision: D10081558

fbshipit-source-id: e4f753feed5b2698dd485cf32403c1c4d15d0c25

Summary:
TSAN complains about a lock inversion between folly/io/async/Request.cpp
lines 242 and 277.  The code as 242 was changed to use folly::acquireLocked() in
D9797715, but the code at 277 needs to be changed in the same way (except for
requesting a write lock on the child)

Reviewed By: kennyyu

Differential Revision: D10251470

fbshipit-source-id: c476a3664bfe83edbddbd402cf568eac885b9123

Summary:
These were failing in the watchman build

```
$ cd build && python fbcode_builder/shell_builder.py > ~/run.sh && bash ~/run.sh
Traceback (most recent call last):
  File "fbcode_builder/shell_builder.py", line 110, in <module>
    steps = make_steps(builder)
  File "/home/travis/build/facebook/watchman/build/fbcode_builder/utils.py", line 94, in <lambda>
    steps_for_spec(builder, config['fbcode_builder_spec'](builder))
  File "/home/travis/build/facebook/watchman/build/fbcode_builder/fbcode_builder.py", line 144, in build
    return [self.setup(), self.diagnostics()] + steps
  File "fbcode_builder/shell_builder.py", line 61, in setup
    ).format(ccache_dir=ccache_dir)
  File "/home/travis/build/facebook/watchman/build/fbcode_builder/shell_quoting.py", line 64, in format
    (k, shell_quote(v).do_not_use_raw_str) for k, v in kwargs.items()
KeyError: u'CC'
```

Reviewed By: snarkmaster

fbshipit-source-id: 614723e631a82f277739765a920731c872700c45

Summary: Add a simple function to dispatch the `instructions` types based on current CPU (overridable in tests).

Reviewed By: luciang

Differential Revision: D10244149

fbshipit-source-id: ac0c1ff0d14402fa77c939361398db6122dea728

Summary: For determining proper use of `strerror_r` based on available decl, move messy `#if` logic out of `String.cpp` and into buck config.

Reviewed By: yfeldblum

Differential Revision: D10243924

fbshipit-source-id: 6c10fc43562765c158d13ed74961277b633bb606

Summary:
- LLVM 7 emits a warning about self-assignment.
- Silence the warning by using a static_cast.
Pull Request resolved: https://github.com/facebook/folly/pull/945

Reviewed By: Orvid

Differential Revision: D10236656

Pulled By: yfeldblum

fbshipit-source-id: 43b46563357d89db9e0911b894f8fead0a14d660

Summary:
Apply fixes that work with new unified headers and remain compatible with old per-API+arch headers.

I do this to unclutter the diff that enables unified headers, so that it only contains things that are incompatible with old per-API+arch headers.

Reviewed By: yfeldblum

Differential Revision: D10225372

fbshipit-source-id: b92cb3ef45cf23f2233c39520236c4531dba715b

Summary:
Use `FOLLY_ATTR_WEAK` for the annotate functions.

(Note: this ignores all push blocking failures!)

Differential Revision: D10234953

fbshipit-source-id: 534cb1d2d7d6053bc000e4c7ae3c6df16fc7a958

Summary:
`shell_builder.py` allows running the fbcode_builder logic
on the host rather than in a container.

It emits a bash script with `set -exo pipefail` configured such that
any failing step will cause the script to exit with failure.

Refs: https://github.com/facebook/watchman/pull/639

Reviewed By: simpkins

Differential Revision: D9552411

fbshipit-source-id: c7835deedf07ea342dcb3de61d576a4fb5439985

Summary: As titled, purely for uniformity.

Reviewed By: yfeldblum

Differential Revision: D10098066

fbshipit-source-id: 12d78fcc60805284d3d85f405475e63a85799041

Summary:
This adds a missing feature. Nested exceptions are cool, see examples in https://en.cppreference.com/w/cpp/error/rethrow_if_nested (bottom of the page).

In short, nested exceptions allow people to add "contextual" information to exceptions. Previously we had to write the `try { ew.throw_exception(); } catch (...) { std::throw_with_nested(xxx); }` boilerplate if we used an exception_wrapper (e.g. in futures-oriented code); with this diff, we're able to just run `ew.throw_with_nested(xxx)`.

Reviewed By: yfeldblum

Differential Revision: D10219994

fbshipit-source-id: cdf0fbe8e4608f6c87d326631e8ffb4919c20711

Summary: Pull Request resolved: https://github.com/facebook/folly/pull/944

Reviewed By: Orvid

Differential Revision: D10231693

Pulled By: yfeldblum

fbshipit-source-id: 9c0c5d7c4c1e4d9d9860c71e608e47d661a41fdd

Summary:
icc accepts the syntax, but doesn't emit the definition into the
object file.

Reviewed By: ricklavoie, yfeldblum

Differential Revision: D10228495

fbshipit-source-id: 73c1676deb4963a9c30886414cc1722d44a79214

Summary:
Remove falls to Future::then that take a nullary lambda. This is deprecated and will be deleted.

This change replaces them with thenValue([](auto&&){...}) or similar.

This does not replace all calls to then with thenValue - other forms will be replaced when those forms of then are due for deletion.

Reviewed By: yfeldblum

Differential Revision: D10219070

fbshipit-source-id: 13b6f64ee2d7c8741537fe1131bdf3b251361413

Summary:
Similar to `--dynamic_cputhreadpoolexecutor`, this is a temporary
kill switch until all outstanding issues are addressed that would allow us
to use dynamic thread pools unconditionally.

Reviewed By: djwatson

Differential Revision: D10231230

fbshipit-source-id: b0abd77ff2a6a7ce31727e14eca799a6f3b5535a

Summary:
There were some read-modify-write operations that were behaving as read and
then modify.  This is incorrect when you use DeterministicAtomic without a
backing schedule.

This is okay if the test is operating in the presence of a
DeterministicSchedule schedule, but is probably worth fixing anyway.  If using
DeterministicAtomic without a backing schedule is considered a bug, we can add
an assertion in beforeSharedAccess() and afterSharedAccess() so users know not
to try that

Reviewed By: djwatson, nbronson

Differential Revision: D10190682

fbshipit-source-id: 6a3173d904a40bbc5c96b934a76aff90a48c8608

Summary:
icc gets confused on the parameter unpacking otherwise.

https://godbolt.org/z/rSRZYn

Reviewed By: yfeldblum

Differential Revision: D10147211

fbshipit-source-id: 85de581ffa4c487523a5e9f173636f74863a82ac

Summary:
When they come before the explicit instantiations of Rob, icc fails to
compile certain combintions of calls.

Reviewed By: alexeyt

Differential Revision: D10144943

fbshipit-source-id: ce63a4d289f64d1d644b116ab99b45b9da013162

Summary:
`folly::SharedMutex` allows `unlock_shared()` to be called in a different thread from the one that locked it
There are some places in folly that use this property.

Example: In `ObserverManager`, the calling thread

1. acquires the read lock
2. then it schedules some async work and gives ownership of the lock guard to the lambda
3. the thread that performs the async work will unlock the mutex

However, this causes false positives for TSAN. It looks like this breaks TSAN's assumptions that a read-write mutex is always read locked and unlocked in the same thread.
In the example above, TSAN thinks the calling thread in step (1) still owns the mutex, even though semantically the mutex is now owned by the thread in step (3).

This results in false positives for running unit tests with an annotated version of `folly::SharedMutex`.
TSAN reports a lock inversion between the `folly::Singleton vaule_.state_` mutex, and the read lock referenced in `ObserverManager`.

To suppress this error, this change annotates ObserverManager to let TSAN know that the the thread in step 1 has dropped
the lock, and the thread in step 3 now owns the lock.

Reviewed By: andriigrynenko

Differential Revision: D9797937

fbshipit-source-id: c0e5277bb9cb2f404df8abd3c3d6ea4a16460c78

Summary:
TSAN flagged the following lock inversion below. The reason for this lock inversion
is because of the following example usage:

1. set request context B, save old one A (lock B, lock A)
2. later on, reset request context back to A, get the old one back B (lock A, lock B)

In the two calls, we always locked the the new context first, resulting
in a lock inversion. The fix is to always acquire them in a consistent
order using `folly::acquireLocked`

TSAN report:

```
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=1574307)
  Cycle in lock order graph: M159 (0x7b1400000138) => M157 (0x7b14000000e8) => M159

  Mutex M157 acquired here while holding mutex M159 in main thread:
    #0 0x48f18c in AnnotateRWLockAcquired ??:?
    #1 0x315495 in folly::SharedMutexImpl<false, void, std::atomic, false>::lock_shared() ./folly/SharedMutex.h:385
    #2 0x1b0eb8 in folly::detail::LockTraitsImpl<folly::SharedMutexImpl<false, void, std::atomic, false>, (folly::detail::MutexLevel)1, false>::lock_shared(folly::SharedMutexImpl<false, void, std::atomic, false>&) ./folly/LockTraits.h:157
    #3 0x1b0e68 in std::integral_constant<bool, true> folly::LockPolicyShared::lock<folly::SharedMutexImpl<false, void, std::atomic, false> >(folly::SharedMutexImpl<false, void, std::atomic, false>&) ./folly/LockTraits.h:493
    #4 0x1d63a9 in LockedPtrBase ./folly/Synchronized.h:1026
    #5 0x1d6258 in LockedPtr ./folly/Synchronized.h:1326
    #6 0x1c9a17 in folly::SynchronizedBase<folly::Synchronized<folly::RequestContext::State, folly::SharedMutexImpl<false, void, std::atomic, false> >, (folly::detail::MutexLevel)1>::rlock() const ./folly/Synchronized.h:123
    #7 0x1c7530 in folly::RequestContext::setContext(std::shared_ptr<folly::RequestContext>) ./folly/io/async/Request.cpp:243
    #8 0x36a7b in folly::RequestContext::create() ./folly/io/async/Request.h:119
    #9 0x34916 in RequestContextScopeGuard ./folly/io/async/Request.h:269

  Mutex M159 previously acquired by the same thread here:
    #0 0x48f18c in AnnotateRWLockAcquired ??:?
    #1 0x315495 in folly::SharedMutexImpl<false, void, std::atomic, false>::lock_shared() ./folly/SharedMutex.h:385
    #2 0x1b0eb8 in folly::detail::LockTraitsImpl<folly::SharedMutexImpl<false, void, std::atomic, false>, (folly::detail::MutexLevel)1, false>::lock_shared(folly::SharedMutexImpl<false, void, std::atomic, false>&) ./folly/LockTraits.h:157
    #3 0x1b0e68 in std::integral_constant<bool, true> folly::LockPolicyShared::lock<folly::SharedMutexImpl<false, void, std::atomic, false> >(folly::SharedMutexImpl<false, void, std::atomic, false>&) ./folly/LockTraits.h:493
    #4 0x1d63a9 in LockedPtrBase ./folly/Synchronized.h:1026
    #5 0x1d6258 in LockedPtr ./folly/Synchronized.h:1326
    #6 0x1c9a17 in folly::SynchronizedBase<folly::Synchronized<folly::RequestContext::State, folly::SharedMutexImpl<false, void, std::atomic, false> >, (folly::detail::MutexLevel)1>::rlock() const ./folly/Synchronized.h:123
    #7 0x1c7516 in folly::RequestContext::setContext(std::shared_ptr<folly::RequestContext>) ./folly/io/async/Request.cpp:242
    #8 0x36a7b in folly::RequestContext::create() ./folly/io/async/Request.h:119
    #9 0x34916 in RequestContextScopeGuard ./folly/io/async/Request.h:269

  Mutex M159 acquired here while holding mutex M157 in main thread:
    #0 0x48f18c in AnnotateRWLockAcquired ??:?
    #1 0x315495 in folly::SharedMutexImpl<false, void, std::atomic, false>::lock_shared() ./folly/SharedMutex.h:385
    #2 0x1b0eb8 in folly::detail::LockTraitsImpl<folly::SharedMutexImpl<false, void, std::atomic, false>, (folly::detail::MutexLevel)1, false>::lock_shared(folly::SharedMutexImpl<false, void, std::atomic, false>&) ./folly/LockTraits.h:157
    #3 0x1b0e68 in std::integral_constant<bool, true> folly::LockPolicyShared::lock<folly::SharedMutexImpl<false, void, std::atomic, false> >(folly::SharedMutexImpl<false, void, std::atomic, false>&) ./folly/LockTraits.h:493
    #4 0x1d63a9 in LockedPtrBase ./folly/Synchronized.h:1026
    #5 0x1d6258 in LockedPtr ./folly/Synchronized.h:1326
    #6 0x1c9a17 in folly::SynchronizedBase<folly::Synchronized<folly::RequestContext::State, folly::SharedMutexImpl<false, void, std::atomic, false> >, (folly::detail::MutexLevel)1>::rlock() const ./folly/Synchronized.h:123
    #7 0x1c7530 in folly::RequestContext::setContext(std::shared_ptr<folly::RequestContext>) ./folly/io/async/Request.cpp:243
    #8 0x34b99 in ~RequestContextScopeGuard ./folly/io/async/Request.h:278

  Mutex M157 previously acquired by the same thread here:
    #0 0x48f18c in AnnotateRWLockAcquired ??:?
    #1 0x315495 in folly::SharedMutexImpl<false, void, std::atomic, false>::lock_shared() ./folly/SharedMutex.h:385
    #2 0x1b0eb8 in folly::detail::LockTraitsImpl<folly::SharedMutexImpl<false, void, std::atomic, false>, (folly::detail::MutexLevel)1, false>::lock_shared(folly::SharedMutexImpl<false, void, std::atomic, false>&) ./folly/LockTraits.h:157
    #3 0x1b0e68 in std::integral_constant<bool, true> folly::LockPolicyShared::lock<folly::SharedMutexImpl<false, void, std::atomic, false> >(folly::SharedMutexImpl<false, void, std::atomic, false>&) ./folly/LockTraits.h:493
    #4 0x1d63a9 in LockedPtrBase ./folly/Synchronized.h:1026
    #5 0x1d6258 in LockedPtr ./folly/Synchronized.h:1326
    #6 0x1c9a17 in folly::SynchronizedBase<folly::Synchronized<folly::RequestContext::State, folly::SharedMutexImpl<false, void, std::atomic, false> >, (folly::detail::MutexLevel)1>::rlock() const ./folly/Synchronized.h:123
    #7 0x1c7516 in folly::RequestContext::setContext(std::shared_ptr<folly::RequestContext>) ./folly/io/async/Request.cpp:242
    #8 0x34b99 in ~RequestContextScopeGuard ./folly/io/async/Request.h:278

```

Reviewed By: igorsugak

Differential Revision: D9797715

fbshipit-source-id: a0f991ae0ec8f3e9d33af6e05db49eceaf3a5174

Summary:
There is a lock inversion between VirtualEventBase and the fibers library:

1. During program shutdown, `GlobalCache::getImpl` acquires its `mutex_`, then it calls `VirtualEventBase::runOnDestruction` which acquires the SharedMutex on `onDestructionCallbacks_`
2. During normal program execution, `VirtualEventBase::destroyImpl()` acquires the `onDestructionCallbacks_` lock and then invokes the user-supplied callbacks, which will invoke `GlobalCache::eraseImpl` and acquire `mutex_`

The reason for this lock inversion is because in step 2, we are holding a mutex while invoking user-supplied callbacks.
The fix is to NOT hold the lock while we invoke these callbacks.

Reviewed By: andriigrynenko

Differential Revision: D9797816

fbshipit-source-id: 861a915f22138f0c8d3f8bca4d7bf4b9a0aa3d26

Summary:
When running tests with a TSAN-annotated version of folly::SharedMutex,
I discovered at least 3 types of lock inversions in folly::Singleton code.
These inversions are probably benign and deadlock is not possible, but we need to
suppress these inversions to prevent too much noise from TSAN.

Lock inversions:

1. The users can supply arbitrary create() functions for Singletons. If the create() function
   happens to acquire a mutex, then this introduces a lock ordering from
   SingletonVault -> mutex. Later on, when the user code runs in normal operations, we might
   hold the mutex and try to create a singleton, resulting in the mutex -> SingletonVault ordering
2. SingletonVault::destroyInstances() holds a mutex, and then invokes the user-supplied destroy()
   functions. In the user supplied destroy functions, we might attempt to create a singleton and
   acquire the lock in SingletonHolder, resulting in SingletonVault -> SingletonHolder lock ordering.
   In normal operations, whenever a singleton is created, we have the SingletonHolder -> SingletonVault
   lock ordering in createInstance()
3. doEagerInit() holds the eagerInitSingleton mutex and then acquires the SingletonVault lock
   during createInstance(). addEagerInitSingleton() holds the SingletonVault lock and then acquires
   the eagerInitSingleton lock.

The source of these errors is that we are invoking user-supplied callbacks while holding mutexes.
However, these lock inversions cannot actually deadlock based on higher-level knowledge of folly::Singleton
state transitions.

To suppress these errors, use the new `folly::SharedMutexSuppressTSAN` mutex instead, which will
not be annotated by TSAN. If we are not building with TSAN, then this mutex is equivalent to a normal
`folly::SharedMutex`.

Reviewed By: andriigrynenko

Differential Revision: D9797988

fbshipit-source-id: 82b5850fe189f7d1dcaca0e3562fabcfafd0cef5

Summary:
This diff adds TSAN annotations to `folly::SharedMutex` to allow TSAN to find
lock inversions with this type of mutex. Note that only the WritePriority version
is annotated, and the ReadPriority version is not. See the comments in the source code
for an explanation of this.

Some notes about how the annotation was done:

- We always call the RELEASED annotations at the beginning of unlock(), and the ACQUIRED annotations at the end of lock().
  This prevents a double locking interleaving where thread 1 has unlocked it and before RELEASED is called, another thread
  has locked it and calls ACQUIRED first.
- These annotations treat upgrade locks as equivalent to a shared lock. This prevents any false positives, and keeps
  the annotations simple.
- We need to keep the constructor for SharedMutex as `constexpr` to avoid static initialization bugs. As a result, we need
  to lazily annotate creation of the mutex. To do this, this adds an extra bit to the `SharedMutex` state to keep track if
  initialization has been called. In TSAN builds, we have an array of mutexes to enforce that initialization is called
  at most once.
- This diff introduces a new template param AnnotateForThreadSanitizer (default is False). This allows users to use a new
  folly::SharedMutexSuppressTSAN version to avoid lock inversion detection if there are noisy lock inversions.
  However, this should be the exception and not the norm. In normal build modes, this is equivalent to a normal SharedMutex.

Reviewed By: nbronson

Differential Revision: D9677058

fbshipit-source-id: b0f5719a75024937fb81672435fb1c9802f255d7

Summary:
This change is part of a bigger effort to make `folly::SharedMutex`
compatible with TSAN so that TSAN can find lock inversions with these
types of mutexes.

In order to do this, we need to annotate the mutex function calls
accordingly. The annotation functions are provided by the TSAN
runtime library.

Upcoming changes to `folly::SharedMutex`:

1. provide annotation helper macros (this change)
2. fix or suppress known issues in existing libraries (these macros will allow us to suppress issues)
3. once all the issues are fixed, annotate `SharedMutex` accordingly

Reviewed By: nbronson

Differential Revision: D9797582

fbshipit-source-id: 12306db98505fe31a8d8590113d10585840fbe6d

Summary:
`AccessSpreader::cachedCurrent()` caches the result of `AccessSpreader::getcpuFunc()` for 32 calls in a thread-local. The cached function takes 2 ns, where the current call takes 12 ns. This comes at the cost of being imprecise when threads migrate to a new cpu.

I chose 32 as the number of calls because it only has a 10% overhead over never refreshing (2.05 ns vs 1.83 ns), where 16 has a 30% overhead, and it performs just as well as 64.

Reviewed By: ot

Differential Revision: D10151009

fbshipit-source-id: 07ed292dfdcdcedcb74c24279f7773a80ad09348

Summary:
* The atomic load was being optimized out of the benchmark.
* The initializer wasn't being called for the test tags, so `AccessSpreader<ThreadLocalTag>::current()` and `AccessSpreader<PthreadSelfTag>::current()` were always returning 0.

Reviewed By: ot

Differential Revision: D10154155

fbshipit-source-id: 76be9865c9ee591d24aedc64e6faf4fd24350282

Summary: ALPN is the preferred way to do this.  We should only use that and not provide NPN support anymore.

Reviewed By: knekritz

Differential Revision: D10110626

fbshipit-source-id: 5ec91fe51252bed0a8d8047e481df76a395e1286

Summary:
Two things
- Use thenValue instead of then in one specific occurrence as later one has been deprecated
- Build googletest on `release-1.8.1` as it is the one that compiles with c++11

Reviewed By: cenzhao

Differential Revision: D10178139

fbshipit-source-id: 069578716e5e55ef0a25ea87e9ef5ceded1ff9d6

Summary: There's no need for this to be inheritable

Reviewed By: yfeldblum

Differential Revision: D10139559

fbshipit-source-id: ae2db0d13e33c383fd73c0563b127e62e54a7094

Summary: Add IOBuf::takeOwnershipIov method

Reviewed By: yfeldblum, djwatson

Differential Revision: D10135962

fbshipit-source-id: 86a22d73aa1b72645640cf026d2dc2f38b02989c

Summary:
Use heterogenous lookup for dynamic::getDefault for
StringPiece/dynamic keys.

Reviewed By: shixiao

Differential Revision: D10127452

fbshipit-source-id: 3cf28a28373211428ac5352136cf073e6c377b16

Summary:
dynamic::setDefault already calls heterogenous `emplace(...)`, however it does not behave correctly for static constexpr strings.
Add a specialization for StringPiece which solves this.

The current set of tests does not invoke setDefault with an object as a key.
This diffs adds a test that does that.

Reviewed By: shixiao

Differential Revision: D10007262

fbshipit-source-id: 386a44bf6b0a24f81e843695d177d85b280b907d

Summary: Document that insert may evaluate its arguments after rehashing.

Reviewed By: shixiao

Differential Revision: D10132409

fbshipit-source-id: ba5996a309ae201ae08808272edd1fb0b981cc64