1. 10 Sep, 2019 2 commits
    • Yedidya Feldblum's avatar
      No mutable flags in IOBuf · bbd6fb43
      Yedidya Feldblum authored
      Summary: [Folly] No mutable flags in IOBuf to avoid races when invoking `const`-qualified members concurrently.
      
      Reviewed By: simpkins
      
      Differential Revision: D17251909
      
      fbshipit-source-id: e52d333fb620d05f7229a11db9c2f14fe9f6f028
      bbd6fb43
    • Yedidya Feldblum's avatar
      Avoid ambiguous call in futures · b1233937
      Yedidya Feldblum authored
      Summary:
      [Folly] Avoid ambiguous call in futures. Because `std::atomic` is in `namespace std`, unqualified calls to `atomic_compare_exchange_strong_explicit` from within `namespace folly` are ADL-ambiguous between the overloads in `namespace std` and in `namespace folly`.
      
      Also:
      * Fix incorrectly-named prototype.
      * Fix used of `std::atomic<T>::value_type` which may not exist.
      
      Fixes #1221.
      
      Reviewed By: aary, Orvid
      
      Differential Revision: D17256763
      
      fbshipit-source-id: ffcaf171e6257cdef29de0ffb63f310d7f4a93eb
      b1233937
  2. 09 Sep, 2019 2 commits
    • Zhenyu Zhou's avatar
      Add a constructor to folly::Codel · 5ca43e12
      Zhenyu Zhou authored
      Summary: Making a new ctor so that we can pass in the parameters
      
      Reviewed By: yfeldblum
      
      Differential Revision: D17173662
      
      fbshipit-source-id: ab518892694e1bec1179177e7d9477e3a89ff38d
      5ca43e12
    • László Várady's avatar
      Fix tests on Arch Linux (#1218) · b36f6181
      László Várady authored
      Summary:
      Compiling folly on Arch Linux triggered a few platform-specific behaviors that were not covered correctly by tests.
      
      - Fix F14{Map,Set} maxSize test cases
          The maximum size of `F14Table` is dependent of its allocator's max_size:
      
          ```
          max_size() -> std::min<std::size_t>(
              (std::numeric_limits<InternalSizeType>::max)(),
              AllocTraits::max_size(a)
          )
          ```
      
          This commit fixes the `F14{Map,Set}` maxSize tests, so they won't fail
          on platforms where the standard allocator's `max_size()` is based on
          `ptrdiff_t` (signed).
      
      -  Fix TestUtilTest
          If glog is compiled without gflags support, `SetCommandLineOptionWithMode()`
          can't force glog to use stderr.
      
          glog has its own compatibility layer for `FLAGS_*` in case gflags is missing,
          that's why `FLAGS_logtostderr` is always available.
      
      - Skip AsyncIOTest tests if O_DIRECT is not supported by the file system
          `fs::temp_directory_path()` returns `"/tmp"` on Linux.
      
          When using systemd, the /tmp directory is mounted by default as tmpfs;
          and tmpfs does not support the `O_DIRECT` flag.
      
          This commit skips tests in case the file can't be opened with `O_DIRECT`.
      Pull Request resolved: https://github.com/facebook/folly/pull/1218
      
      Reviewed By: nbronson
      
      Differential Revision: D17160597
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: 00274d1f8770079e35559ee9bd8426402e949abd
      b36f6181
  3. 08 Sep, 2019 1 commit
    • Yedidya Feldblum's avatar
      Switch from -defs.h pattern · f5502bfd
      Yedidya Feldblum authored
      Summary: [Folly] Switch from `-defs.h` pattern to `-inl.h` pattern in `folly/stats/` since `extern template` doing shallow instantiation rather than deep instantiation makes this pattern difficult.
      
      Reviewed By: chadaustin
      
      Differential Revision: D16761319
      
      fbshipit-source-id: 1760bf53b09791bebf996064c0d1fcc3babf43d2
      f5502bfd
  4. 07 Sep, 2019 1 commit
    • Yedidya Feldblum's avatar
      Bake templates into the compiler binary · 355677ed
      Yedidya Feldblum authored
      Summary: [Thrift] Bake templates into the compiler binary rather than having to package the templates with the compiler.
      
      Reviewed By: stevegury, vitaut
      
      Differential Revision: D16356056
      
      fbshipit-source-id: b971dce7aada8dc2a26587fbf818e661acd0c555
      355677ed
  5. 06 Sep, 2019 4 commits
    • Yedidya Feldblum's avatar
      Extract compare-exchange hack under TSAN · 2ccb83e2
      Yedidya Feldblum authored
      Summary:
      [Folly] Extract compare-exchange hack under TSAN to a common location and make its interface mimic `std::atomic_compare_exchange_strong_explicit`. Limit it to Clang TSAN (no knowledge of whether it applies to other implementations of TSAN), but extend it to handle all pairs of success and failure orders.
      
      The TSAN bug is described in https://github.com/google/sanitizers/issues/970. High level, Clang TSAN ignores the explicit failure order and infers a failure order from the success order - which is broken if the explicit failure order is in the relevant sense stronger than the success order.
      
      Reviewed By: aary, nbronson
      
      Differential Revision: D16854217
      
      fbshipit-source-id: 18f6458520bbd5f482e41c10d7229e6cfae1db2a
      2ccb83e2
    • Wez Furlong's avatar
      getdeps: build all available boost libraries · ff5e2117
      Wez Furlong authored
      Summary:
      This should make it easier to eg: enable fbzmq on macos at a later
      time, and also makes things more explicit about what is being built, and
      importantly, by listing these in the manifest ensures that we have a
      hash change if we change this list; we wouldn't trigger such a change
      if the list were encoded solely in builder.py
      
      Reviewed By: chadaustin
      
      Differential Revision: D17133149
      
      fbshipit-source-id: caf0dd45e262188eeefafe0868ef95ad257a4950
      ff5e2117
    • Wez Furlong's avatar
      getdeps: allow aliasing sources for add_fb_python_executable · 743cae42
      Wez Furlong authored
      Summary:
      * Fix a minor issue with `add_fb_python_executable` where it would
        emit `--main` even if `MAIN_MODULE` had not been specified
      * Introduce the ability to rename python sources in the manifest so
        that they have a different install path.
      * Note that this changes the DEPENDS parameters to reference abs_sources;
        previously these were using relative paths to the sources pulled directly
        from the SOURCES arguments, but since those can now contain aliasing
        operations we need to pre-process the list.  I switched to abs_sources
        because that was already being maintained.  If for some reason the
        DEPENDS list needs to be relative paths, this will need to be adjusted.
      
      Reviewed By: simpkins
      
      Differential Revision: D17147417
      
      fbshipit-source-id: d0825bfbdad6be658c078aaac6645e6926a1530f
      743cae42
    • Wez Furlong's avatar
      getdeps: add build cache abstraction · f40148ce
      Wez Furlong authored
      Summary:
      This diff adds a small abstraction that allows for uploading
      and downloading from an artifact cache.
      
      We try to download from this cache at build time, but will only
      try to populate it for continuous builds--those are built from
      code that has been reviewed and landed on master.  This restriction
      helps to avoid thrashing the cache with works in progress and
      results in a slightly more trustworthy state of the cache contents.
      
      In addition to this, we choose only to cache third party projects.
      The rationale is that our first party projects move too quickly to
      be worth caching, especially since the cache granularity is for
      the whole project rather than just changed elements of a given
      project.
      
      In a later diff I will introduce some implementations of the
      cache class that work with eg: Travis or Circle CI caching.
      
      Reviewed By: simpkins
      
      Differential Revision: D16873307
      
      fbshipit-source-id: 2bfb69e36615791747b499073586562f2ca48be9
      f40148ce
  6. 05 Sep, 2019 1 commit
    • Lee Howes's avatar
      Increase timeout to a length that is unlikely to trigger · f20cc52f
      Lee Howes authored
      Summary: one_ms is short enough that it can trigger under testing. Increase to make less likely to fail spuriously.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D17190402
      
      fbshipit-source-id: 091cd3ee8f3da224e39ba95c403e233115a80510
      f20cc52f
  7. 04 Sep, 2019 5 commits
    • Lee Howes's avatar
      Fix documentation of Future::within and SemiFuture::within · 63d655cf
      Lee Howes authored
      Summary: SemiFuture::within was undocumented and Future::within's use of the term "throws" was confusing. This cleans up both sets of documentation.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D17186392
      
      fbshipit-source-id: 83409df85b07bd3cad9fb020e99fc3a7f3c70b61
      63d655cf
    • Dan Melnic's avatar
      Disable FOLLY_TLS under 32 bit Apple/iOS · 67dc542a
      Dan Melnic authored
      Summary: Disable FOLLY_TLS under 32 bit Apple/iOS
      
      Reviewed By: mzlee
      
      Differential Revision: D17172252
      
      fbshipit-source-id: ab47bb9ddb810aec45a940fc2ae834bac6a48f7c
      67dc542a
    • László Várady's avatar
      Remove std::move() calls that prevent NRVO (#1217) · ee3a3a1e
      László Várady authored
      Summary:
      > Under the following circumstances, the compilers are permitted to omit the copy and move construction of class objects:
      
      > In a return statement, when the operand is the name of a non-volatile object with automatic storage duration, which isn't a function parameter or a catch clause parameter, and which is of the same class type (ignoring cv-qualification) as the function return type. This variant of copy elision is known as NRVO, "named return value optimization".
      
      `std::move()` prevents this optimization.
      
      GCC 9.1.0 warning:
      ```
      moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
      ```
      Pull Request resolved: https://github.com/facebook/folly/pull/1217
      
      Reviewed By: LeeHowes
      
      Differential Revision: D17173234
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: ed63ef89dfd01883ddd6fd775285997f96a029fd
      ee3a3a1e
    • Yedidya Feldblum's avatar
      Cut mstch manifest · eea51e2d
      Yedidya Feldblum authored
      Summary: [OpenSource] Cut `mstch` manifest, which was but is no longer used by `fbthrift`.
      
      Reviewed By: avalonalex, davidtgoldblatt
      
      Differential Revision: D17160816
      
      fbshipit-source-id: 3b96142455d6990870e8ed3e5ce330d339d5a657
      eea51e2d
    • Wez Furlong's avatar
      getdeps: assign test owner in continuous runs · 241df17c
      Wez Furlong authored
      Summary:
      Feed the first-party oncall data through to the testpilot
      invocation.  This will set the owner of the test in continuous runs.
      
      The oncall is passed through via the sandcastle module, which means
      that you will have to manually pass it through for local testing
      (see test plan), but it should automatically get picked up when
      sandcastle schedules the `test` step of the job.
      
      Reviewed By: chadaustin
      
      Differential Revision: D17146802
      
      fbshipit-source-id: a4f0e65853a46ed8709594c96db859ede2530b00
      241df17c
  8. 03 Sep, 2019 6 commits
    • John Strizich's avatar
      add generated constant definitions to library · 7b373430
      John Strizich authored
      Summary: when trying to build openr with `getDeps.py` constant definitions where not available since they were not part of the generated thrift lib. this fixes it.
      
      Reviewed By: chadaustin
      
      Differential Revision: D16064417
      
      fbshipit-source-id: ee2b3e061caf67235f4a873405cf6cd6fb663ef4
      7b373430
    • Kirk Shoop's avatar
      Add folly::coro::multiplex() algorithm · b963f391
      Kirk Shoop authored
      Summary: This algorithm multiplexes values from a stream-of-streams into a single stream of events that identifies the stream index the event originated from and also the end-of-stream and stream-error events.
      
      Reviewed By: lewissbaker
      
      Differential Revision: D15643142
      
      fbshipit-source-id: a309a485f11465d9068ea81f4bc32823dd34291f
      b963f391
    • Kirk Shoop's avatar
      Add folly::coro::transform() algorithm · 600a1046
      Kirk Shoop authored
      Summary: The `transform()` algorithm uses the provided function to transform each value in the provided input stream and produces each transformed value on the returned output stream.
      
      Reviewed By: lewissbaker
      
      Differential Revision: D16774893
      
      fbshipit-source-id: 7ab33706c667b1706c0cb0b1c0ecb9046f47a325
      600a1046
    • Kirk Shoop's avatar
      Add folly::coro::merge() operator for AsyncGenerator. · 189c99a6
      Kirk Shoop authored
      Summary: Adds a new `folly::coro::merge()` algorithm that accepts an async stream of async streams and returns an async stream that merges and interleaves the results.
      
      Reviewed By: lewissbaker
      
      Differential Revision: D15617383
      
      fbshipit-source-id: cafb8e31725cdb21cfa00ce186901329828c2b68
      189c99a6
    • Luca Niccolini's avatar
      BUILD_SHARED_LIBS=OFF for CI build · 6e4965ce
      Luca Niccolini authored
      Summary: fixes the CI build
      
      Reviewed By: dddmello
      
      Differential Revision: D17158685
      
      fbshipit-source-id: 44ea871eab4d8c964b8372fb8d84879fa823ca50
      6e4965ce
    • Eric Niebler's avatar
      Update fbcode for range-v3 0.9.1-pre · 3685bdd1
      Eric Niebler authored
      Summary:
      Range-v3 v0.9.1-pre deprecated and renamed a bunch of interfaces to harmonize with the direction of C++20's `std::ranges`. In this diff, I:
      
      * Rename the `ranges::view` namespace qualification to `ranges::views` (plural),
      * Rename the `ranges::action` namespace qualification to `ranges::actions` (plural),
      * Rename `ranges::view::bounded` to `ranges::views::common`,
      * Rename `ranges::to_` to `ranges::to`,
      * Replace _some_ implicit conversions of views to containers with explicit invocations of `ranges::to` (more work to be done here, but can wait).
      
      Reviewed By: mzlee
      
      Differential Revision: D17061465
      
      fbshipit-source-id: f28205ecf3965d3560e9cffe2805d0b404597b21
      3685bdd1
  9. 02 Sep, 2019 1 commit
    • Lee Howes's avatar
      Remove executor-taking form of then · 2d5ad1d0
      Lee Howes authored
      Summary:
      Remove calls to Future::then(executor, callback).
      
      This form of Future::then is ambiguous, and does not yet implement the stronger
      typing of thenValue and thenTry. It is also tempting to use instead of via,
      where it is not obvious that it has the behaviour of wrapping a via call in a
      push and pop of the current executor:
      .pushCurrentExecutor().via(executor).then(callback).popCurrentExecutor().
      
      With the addition of inline continuations, we can instead make the nesting
      explicit at low cost by making it an inline continuation that launches an
      asynchronous task on the passed executor.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D15100419
      
      fbshipit-source-id: e929e2eaf2fc30313cb099150be3a9535ae729a6
      2d5ad1d0
  10. 01 Sep, 2019 1 commit
    • Lee Howes's avatar
      Make folly::TimeKeeper::after and ::at return SemiFuture · 11003288
      Lee Howes authored
      Summary:
      TimeKeeper::after and TimeKeeper::at return Futures that for the majority of implementations complete on a TimeKeeper owned executor. This can lead to problems.
      
      Consistent with other API cleanups, this diff switches TimeKeeper APIs to return SemiFuture, requiring that the caller attach an explicit executor when work is to be scheduled downstream of the TimeKeeper.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D17005944
      
      fbshipit-source-id: 98d7e8aaf1ad40ce2180503f632f2f0003bb12be
      11003288
  11. 31 Aug, 2019 3 commits
    • Lee Howes's avatar
      Add result DeferredExecutor to avoid risk of double nesting · eb5c5900
      Lee Howes authored
      Summary: Under some circumstances the DeferredExecutor might already have nested executors and this code adds more. Both incoming futures should instead nest within a fresh DeferredExecutor returned that is part of the SemiFuture from the operation.
      
      Reviewed By: mpark
      
      Differential Revision: D17121117
      
      fbshipit-source-id: b4507bf5d7b0c98e5e5de6f73ff4bd3e2790374c
      eb5c5900
    • Yedidya Feldblum's avatar
      RTM re-nits for xabort · 5db275b9
      Yedidya Feldblum authored
      Summary:
      [Folly] RTM re-nits for `xabort`.
      
      * Remove param pack.
      * Always noreturn.
      
      Reviewed By: nbronson
      
      Differential Revision: D17117350
      
      fbshipit-source-id: b16056b1f02956d334da4494c689a4b988a961d4
      5db275b9
    • Kirk Shoop's avatar
      Add folly::coro::materialize() & folly::coro::dematerialize() algorithms · 37a33e67
      Kirk Shoop authored
      Summary:
      The `materialize()` algorithm meterializes values from a stream into a stream of events that represent each value and also the end-of-stream and stream-error events.
      
      `dematerialize()` reverses a materialized stream by replaying the events recorded in the input stream to an output stream.
      
      Reviewed By: lewissbaker
      
      Differential Revision: D16772236
      
      fbshipit-source-id: 27de543ea2d02691a376fd0f11d0113c5c39287d
      37a33e67
  12. 30 Aug, 2019 8 commits
    • Wez Furlong's avatar
      fbzmq: disable build on darwin and windows · f3352d86
      Wez Furlong authored
      Summary:
      boost.fiber is not available on macos and is required by fbzmq,
      so disable the build on darwin.
      
      In D17128935 I tried to make libzmq build with cmake to build
      on windows, but there's a problem with its cmake and it fails
      to configure.
      
      So let's just nop out the build for !linux for now.
      
      Reviewed By: jstrizich
      
      Differential Revision: D17129362
      
      fbshipit-source-id: db89010d18707a4105b8d395b1758f738336495d
      f3352d86
    • Paul O'Shannessy's avatar
      Adopt Contributor Covenant · 714c7a17
      Paul O'Shannessy authored
      Summary:
      In order to foster healthy open source communities, we're adopting the
      [Contributor Covenant](https://www.contributor-covenant.org/). It has been
      built by open source community members and represents a shared understanding of
      what is expected from a healthy community.
      
      Reviewed By: josephsavona, danobi, rdzhabarov
      
      Differential Revision: D17104640
      
      fbshipit-source-id: d210000de686c5f0d97d602b50472d5869bc6a49
      714c7a17
    • Adam Simpkins's avatar
      fbcode_builder: add a FBThriftLibrary.cmake file · 37507450
      Adam Simpkins authored
      Summary:
      This module provides a `add_fbthrift_library()` convenience function that can
      be used to generate thrift libraries for multiple languages with a single
      call.
      
      Reviewed By: wez
      
      Differential Revision: D16742275
      
      fbshipit-source-id: c13120cf9e051629951282141eb22f3e085782f9
      37507450
    • Adam Simpkins's avatar
      fbcode_builder: add an FBThriftPyLibrary.cmake module · 2dcce6ca
      Adam Simpkins authored
      Summary:
      This module helps generate Python libraries from thrift files.
      It is the Python equivalent of `FBThriftCppLibrary.cmake`
      
      Reviewed By: wez
      
      Differential Revision: D16742274
      
      fbshipit-source-id: ef0dc7f24eb1c7375d2587ee6b71fd25bd6f35a7
      2dcce6ca
    • Adam Simpkins's avatar
      fbcode_builder: fix a minor bug in THRIFT_INCLUDE_DIR argument parsing · 87f0031a
      Adam Simpkins authored
      Summary:
      Fix a minor bug in the argument parsing for `add_fbthrift_cpp_library()`:
      only a single value should be supplied for the `THRIFT_INCLUDE_DIR` argument.
      This parameter should not accept a list of arguments.
      
      Reviewed By: strager
      
      Differential Revision: D17125810
      
      fbshipit-source-id: 31f354e4d6299d3626a94f3b4b5ff8c58bb0cf7b
      87f0031a
    • Lewis Baker's avatar
      Add CancellationToken support to AsyncGenerator · 50f425bb
      Lewis Baker authored
      Summary:
      An AsyncGenerator coroutine now has an implicit associate CancellationToken that is inherited from the calling context of each `co_await gen.next()` call.
      
      This means that generators that correspond to long-running subscriptions now have an in-built channel that allows them to cancel the subscription.
      
      This is also useful for algorithms that introduce concurrency in processing of stream data. eg. The `buffer()` or `merge()` algorithms.
      
      Reviewed By: kirkshoop
      
      Differential Revision: D16833864
      
      fbshipit-source-id: be8faf1ec438c4248091e02d6a264d3760e2d73d
      50f425bb
    • Uladzislau Paulovich's avatar
      folly | Inline Future constructors to workaround MSVC bug (#1212) · 178fe966
      Uladzislau Paulovich authored
      Summary:
      Pull Request resolved: https://github.com/facebook/folly/pull/1212
      
      MSVC has a bug and can't match definitions of some of the constructors to the corresponding declarations in a class body. This diff workarounds the bug by inlining constructors.
      
      Reviewed By: LeeHowes
      
      Differential Revision: D17119718
      
      fbshipit-source-id: eb7a9d4916b962c718d047eb0ee7fa4a9761776e
      178fe966
    • Lewis Baker's avatar
      Add CancellationToken support to collectAll() · b1ac6a22
      Lewis Baker authored
      Summary:
      The `collectAll()`, `collectAllRange()` and `collectAllWindowed()` functions now cancel their subtasks when either the parent task is cancelled or when any of the subtasks fail with an exception.
      
      The `collectAllTry()`, `collectAllTryRange()` and `collectAllTryWindowed()` functions now cancel their subtasks if the parent task is cancelled, but not if the child tasks fail with an exception.
      
      The logic between the distinction here is that if you are calling one of the 'try' functions then you are intending to handle partial failure and so probably want each of the operations to run to completion independently of the others.
      
      Reviewed By: kirkshoop
      
      Differential Revision: D16820619
      
      fbshipit-source-id: 5c203b41592a0145ff7be1c609dfccd39c41a3df
      b1ac6a22
  13. 29 Aug, 2019 5 commits
    • Uladzislau Paulovich's avatar
      folly | Improve support for clang-cl compiler in folly/Portability.h · 412928c4
      Uladzislau Paulovich authored
      Summary: `clang-cl` on Windows also defines `__PRETTY_FUNCTION__` which has a format different from `__FUNCSIG__`. Therefore folly breaks users of this macro. Alias should be defined only when "real" MSVC compiler is used.
      
      Reviewed By: akrieger, Orvid, blackm00n
      
      Differential Revision: D17094657
      
      fbshipit-source-id: 4b191a61201704382e6b6e178bcc1c673511c537
      412928c4
    • Adam Simpkins's avatar
      fbcode_builder: rename add_thrift_cpp2_library() to add_fbthrift_cpp_library() · 464fac7e
      Adam Simpkins authored
      Summary:
      Rename the `ThriftCppLibrary.cmake` file to `FBThriftCppLibrary.cmake`, and
      also rename `add_thrift_cpp2_library()` to `add_fbthrift_cpp_library()`.
      
      Explicitly calling this `fbthrift` helps clearly distinguish that this is
      intended for use with fbthrift (https://github.com/facebook/fbthrift/), as
      opposed to Apache thrift.
      
      Reviewed By: wez
      
      Differential Revision: D16738440
      
      fbshipit-source-id: 9b255e06b71c98ad74a34989f564a211958dcdd5
      464fac7e
    • Nathan Bronson's avatar
      skip CacheLocality optimizations if there are offline CPUs · e545b9a9
      Nathan Bronson authored
      Summary:
      CacheLocality's parsing code for /proc/cpuinfo doesn't properly
      handle systems in which there are offline CPUs. This diff makes it so
      we fall back onto the sequential cache locality implementation in the
      problematic scenarios.
      
      This should address https://github.com/facebook/folly/issues/1208
      
      Reviewed By: shixiao
      
      Differential Revision: D17093176
      
      fbshipit-source-id: 4da3a9dc53622f829af37ed70a2531c66fd92ddb
      e545b9a9
    • Aaryaman Sagar's avatar
      Use folly::cacheline_align_v for cache alignment in thread pools · 27045b27
      Aaryaman Sagar authored
      Summary:
      Use folly::cacheline_align_v to prevent failures when running folly on platforms
      where extended alignment is not supported.  The thread pools only used extended
      alignment as an optimization; so it is safe to disable it when not needed
      
      Reviewed By: yfeldblum
      
      Differential Revision: D17077439
      
      fbshipit-source-id: 63ba59aaf7ce45231bb25427af84292c3a3464a9
      27045b27
    • Haijun Zhu's avatar
      Fix race condition in fibers CacheManager · 7cfbc254
      Haijun Zhu authored
      Summary:
      The non-atomic field `inUse_` is protected by a lock in `getStackCache`, but not in `giveBack`. This
      is a data race and it is caught by TSAN.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D17109710
      
      fbshipit-source-id: 7c0d5d948740516422efca0895a702566c804bb4
      7cfbc254