1. 25 Jul, 2020 3 commits
    • Misha Shneerson's avatar
      Saturation detector. CPUThreadPoolExecutor integration. Diff 2/7 · a13deea5
      Misha Shneerson authored
      Summary: We should also use saturation detector on queues fronting CPUThreadPoolExecutor.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D20873689
      
      fbshipit-source-id: 25ffc4cf62ddb222ede4f983f88a1c6545a0d4a5
      a13deea5
    • Robin Cheng's avatar
      Fix TSAN for FunctionSchedulerTest. · d3443853
      Robin Cheng authored
      Summary: The pthread_create redefinition also overrides the TSAN-specific pthread_create. This diff deletes this test because the test itself is of little value - see discussion in this diff.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22735273
      
      fbshipit-source-id: a23089654697b85698f226b1110af3ed1a0b7fce
      d3443853
    • Robin Cheng's avatar
      Fix thread leak in //folly/portability/test:pthread_test. · 66e9c658
      Robin Cheng authored
      Summary: The test forgot to join the pthread.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22743458
      
      fbshipit-source-id: 02231ce1e915b9f8679378eaf2ec18dc5dd67280
      66e9c658
  2. 24 Jul, 2020 7 commits
    • Lewis Baker's avatar
      Add retryWhen(), retryN() and retryWithExponentialBackoff() · ba3ee5d7
      Lewis Baker authored
      Summary:
      Add some coroutine-native versions of the retrying algorithms available
      for retrying folly::Future-based operations.
      
      Adds the following:
      * `retryWhen()` - This is the fundamental retry algorithm that allows
        you to specify logic to determine whether to retry and if so when to
        retry.
      * `retryN()` - For simple use-cases that immediately retries operations
        up to N times.
      * `retryWithExponentialBackoff()` - Retries the operation after an
         exponentially increasing delay with some random jitter applied.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22535487
      
      fbshipit-source-id: c5594d0edfc2414015c345bee79e34f5cdafb789
      ba3ee5d7
    • Chad Austin's avatar
      fix link error on gcc 10 · 7daf90c7
      Chad Austin authored
      Summary:
      Building Folly on Fedora 32 fails with a link error, saying
      SingletonVault::kDefaultShutdownTimeout_ is undefined. I did not
      investigate, but this constant is only used in one place, so inline
      it.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22713828
      
      fbshipit-source-id: 707797eaef4633e642d07b2b02d9f55187afb99f
      7daf90c7
    • Lewis Baker's avatar
      Add folly::coro::timeout() algorithm · 008f5538
      Lewis Baker authored
      Summary:
      This algorithm takes an awaitable and a duration and
      cancels the operation and completes with a `FutureTimeout`
      error if the operation does not complete within the specified
      duration.
      
      This differs from `timed_wait()` in that it still waits
      for the child operation to complete, even once the timeout
      has elapsed, rather than just detaching from the
      operation and letting it complete in the background.
      
      This relies on the operation responding to cancellation
      in a timely manner for the algorithm to have the effect
      of returning shortly after the timeout elapses.
      
      The benefit of waiting for the child operation to complete,
      however, is that it allows you to safely pass parameters by
      reference to the child operation and be safe in knowing that
      the child operation will complete before the awaiting
      coroutine resumes and potentially destroys those parameters.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22199932
      
      fbshipit-source-id: 4f6295a23a860ec52b639eb56a02f8029ffb9009
      008f5538
    • Misha Shneerson's avatar
      Saturation detector. Basic algorithm. Diff 1/7 · 586a200d
      Misha Shneerson authored
      Summary: Basic FIFO queues observation hooks.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D20773870
      
      fbshipit-source-id: 3f9cbab1b8b10204139ce153d92e08ffb5f6254c
      586a200d
    • Andrew Gallagher's avatar
      Fix `F14*Fallback.h` headers in modular builds · bddd247e
      Andrew Gallagher authored
      Summary:
      The `F14*Fallback.h` headers are meant to be included textually
      from `F14*.h` headers and currently fail to compile in modular
      builds, as a) they have references to items defined in their intended
      enclosing header and b) even if successfully compiled alone would
      provide conflicting definitions of the `F14*` structures.
      
      This diff adds the `FOLLY_F14_VECTOR_INTRINSICS_AVAILABLE`
      gate in the `F14*Fallback.h` headers so that they won't get compiled
      when they're not used.
      
      Reviewed By: yfeldblum, luciang
      
      Differential Revision: D22568097
      
      fbshipit-source-id: a75b3a4a687e3034dd9c4c3f7fd0a41bfd465272
      bddd247e
    • Andrew Gallagher's avatar
      Fix missing headers exposed by modular builds · 98eb49fa
      Andrew Gallagher authored
      Reviewed By: luciang
      
      Differential Revision: D22555074
      
      fbshipit-source-id: b853730e5d0dd96b6f94d71e2332653a1a44c9a1
      98eb49fa
    • Brandon Schlinker's avatar
      Fix platform build error caused by lack of return · 613428df
      Brandon Schlinker authored
      Summary: Some build environments will complain that there is no return specified for the two modified functions despite the `CHECK(false)`. Switching to `folly::terminate_with` resolves.
      
      Differential Revision: D22659960
      
      fbshipit-source-id: cff7dd1366fab5c2530584b1f57821bc26b672d4
      613428df
  3. 23 Jul, 2020 3 commits
    • Yedidya Feldblum's avatar
      Move makeAsyncTask test helpers inline into the test · ccd77b85
      Yedidya Feldblum authored
      Summary: [Folly] Move `makeAsyncTask` test helpers inline into the test.
      
      Reviewed By: ericniebler
      
      Differential Revision: D22674219
      
      fbshipit-source-id: 630c84ce153ad2bfb92d38f3d2c955cb3f043799
      ccd77b85
    • Andrii Grynenko's avatar
      detachOnCancel: fix shadow-uncaptured-local warning · 6edefe20
      Andrii Grynenko authored
      Summary: (Note: this ignores all push blocking failures!)
      
      Differential Revision: D22684908
      
      fbshipit-source-id: bb00bf2896880ae6ed5f189110bb13b5abbd80cd
      6edefe20
    • Andrii Grynenko's avatar
      Basic detachOnCancel helper · 88ff31c0
      Andrii Grynenko authored
      Summary:
      detachOnCancel adapter can be used with Awaitables that don't support active cancellation, but are safely detachable. When the cancellation is requested the adaptor will emit OperationCancelled exception without waiting for the awaitable to complete.
      
      This diff also introduces tryAssign helper, similar to Try::operator=, but which handles a case of throwing move constructor.
      
      Reviewed By: lewissbaker
      
      Differential Revision: D22578847
      
      fbshipit-source-id: cab9998c8c6a709108edefc3d59e6f44792d562f
      88ff31c0
  4. 22 Jul, 2020 2 commits
    • Yedidya Feldblum's avatar
      Add a missing forward in makeAsyncTask · 27a0093e
      Yedidya Feldblum authored
      Summary:
      [Folly] Add a missing `std::forward` in `makeAsyncTask`.
      
      As a matter of policy in folly futures, continuations are invoked with the cv-ref qualifiers (excluding v qualifiers) with which they are passed. Here is a case where the policy was not upheld.
      
      Reviewed By: ericniebler
      
      Differential Revision: D22672148
      
      fbshipit-source-id: b8dba221914221d390617fa1c69736c5c547e40c
      27a0093e
    • Pranav Thulasiram Bhat's avatar
      Pass rootId in SDT for shallow copied RCs · 85009b3f
      Pranav Thulasiram Bhat authored
      Summary:
      The shallow copied child RC is often used to make small changes to the RC for a limited scope of the request.
      
      The tracepoint however reports the raw address of the RC as a way to identify the request itself - Therefore we end up reporting different identifiers for what is 'logically' the same request.
      
      This diff adds two additional arguments (the rootIds or pointers to the parent RC) to allow bpf programs to pick up this subtlety.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22424318
      
      fbshipit-source-id: f669cabca10fa50b8a81fb4b3585d30654207e4c
      85009b3f
  5. 21 Jul, 2020 7 commits
    • Nathan Bronson's avatar
      add heterogeneous lookup and eraseInto to fallback F14 sets · 74e8af86
      Nathan Bronson authored
      Summary:
      This diff adds heterogeneous lookup support and eraseInto to the
      fallback (non-SIMD) version of F14 sets, which is used on mobile.
      std::is_invocable<hasher, const_iterator const&> is no longer evaluated
      during erase.  It also restructures F14MapTest and F14SetTest so that
      as many of the tests as possible run in the fallback mode.
      
      (Note: this ignores all push blocking failures!)
      
      Reviewed By: yfeldblum, ot
      
      Differential Revision: D21489440
      
      fbshipit-source-id: 872a250f8a6ec3c9efcd19edb7d9beca196fa865
      74e8af86
    • Nathan Bronson's avatar
      heterogeneous lookup for F14 map fallback · ce0007aa
      Nathan Bronson authored
      Summary:
      This diff implements efficient heterogeneous lookup, insert,
      and erase for the fallback (non-SIMD) mode of F14.  It relies on internal
      implementation details of the underlying std::unordered_map implementation
      that happen to be true for libstdc++, libc++, and MSVC.
      
      Extending this technique to F14 sets would be straightforward, but has
      not been done here.
      
      (Note: this ignores all push blocking failures!)
      
      Differential Revision: D21408628
      
      fbshipit-source-id: 72e9ddd82a50c02ec762d4b64dc3303848bd0218
      ce0007aa
    • Lee Howes's avatar
      Remove Future::getTry · 5d517eb7
      Lee Howes authored
      Summary:
      getTry reads like a blocking operation but is actually non-blocking.
      
      Remove to avoid confusion.
      
      (Note: this ignores all push blocking failures!)
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22494732
      
      fbshipit-source-id: d884d60cc14e2a53cd0657926a885a12b994c1ee
      5d517eb7
    • Robin Cheng's avatar
      Fix a TSAN-reported race condition in ThreadLocal. · 2e45ff8d
      Robin Cheng authored
      Summary:
      This is a pretty cryptic race condition, but it works like this:
      
      Each thread local instance (i.e. one ThreadLocalPtr) is assigned a unique ID by the static meta. This ID is freed up when the thread local is destroyed, and may then be reused by another newly created thread local.
      
      For each thread we keep an array of elements (ElementWrapper), indexed by the thread local ID. Each element has some metadata such as the pointer to the object being cached for that thread and that thread local.
      
      There are a few cases where we access a ElementWrapper object:
       - We write to it under a global lock, when destroying a thread local object (only the i-th element is written to for each thread, if the ID is 'i' for the thread local).
       - We write to it under a global lock, when reallocating the array of elements for the thread if a new thread local ID exceeds the capacity of the array; this can happen when we are reading the thread local for a thread.
       - We write to it under a global lock when the corresponding thread is destroyed.
       - We read from it, *without* any locks, when reading a thread local's value from a thread and the above reallocation case does not occur.
      
      The race condition happens in this case:
       - Thread locals A and B are created
       - Thread local A is accessed by thread Z, and gets, say ID = 5, the corresponding element E is initialized
       - Thread local A is destroyed by thread X, and under the global lock, the element E is written to (setting element->ptr = nullptr); ID 5 is released to the list of free IDs.
       - Thread local B is accessed for the first time by thread Y, and therefore needs to grab an ID; under the global lock, it happens to get ID = 5, and stores that ID into the thread local instance as an atomic int; the store here uses std::memory_order_seq_cst.
       - Thread local B is then accessed again from thread Z; this time the atomic int ID is loaded with std::memory_order_relaxed, with the resulting value of 5; we then proceed to read element E's ptr value *without any locks*.
      
      TSAN reports the last read of element E to be racy with the previous write of element E during thread X's destruction of thread local A. Even though this happened with a mutex operation in between, the final std::memory_order_relaxed causes TSAN to think that the read access was not properly synchronized after the earlier write.
      
      This conjecture is confirmed by the following small reproducible test case:
      
      ```
      TEST(MyTest, Test) {
        std::mutex mutex;
        int x = 0;
        bool written = false;
        std::atomic<bool> seen{false};
      
        std::thread t0([&] {
          while (!seen.load(std::memory_order_relaxed)) {
          }
          std::cout << x;
        });
      
        std::thread t1([&] {
          while (true) {
            std::lock_guard<std::mutex> guard(mutex);
            if (written) {
              seen.store(true, std::memory_order_release);
              break;
            }
          }
        });
      
        std::thread t2([&] {
          std::lock_guard<std::mutex> guard(mutex);
          x = 1;
          written = true;
        });
      
        t0.join();
        t1.join();
        t2.join();
      }
      ```
      
      (This example, when run under TSAN, reports a race condition between "x = 1" and "std::cout << x".)
      
      This change makes the load use std::memory_order_acquire so it synchronizes with the store. Should have no performance implications on x86 but I'm not totally confident about that.
      
      While we're at it, also changing the allocate() and destroy() functions to use more relaxed memory orders; sequential consistency is unnecessary in these places.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22561685
      
      fbshipit-source-id: 0d53925486ef78139ffe71681827b0c0959dd29e
      2e45ff8d
    • Nathan Bronson's avatar
      callWithExtractedKey fix for old libstdc++ · 6c21531d
      Nathan Bronson authored
      Summary:
      libstdc++ versions that don't include N4387 ("perfect initialization" for
      pairs and tuples) can't use the two-arg pair constructor as an emplace
      target when the emplace args don't exactly match the destination types.
      (libstdc++.so.6.0.22 is the first one that contains the improvement,
      which corresponds to gcc 6.1.0).  This diff guards the optimization added
      in D21419822 (https://github.com/facebook/folly/commit/7d618e4fde39b44e0952880653e10ec97400583f) with an enable_if test to avoid the problem, and includes
      the template parameter changes necessary to get the target types to the
      test site.
      
      (Note: this ignores all push blocking failures!)
      
      Differential Revision: D21766560
      
      fbshipit-source-id: f8b2a0230504237f976e14b0740bb31bc7a57b77
      6c21531d
    • Yedidya Feldblum's avatar
      A test for the atomic_ref deduction guide · 67ed8482
      Yedidya Feldblum authored
      Summary:
      [Folly] A test for the `atomic_ref` deduction guide, which permits removal of the preprocessor guard in the definition of `make_atomic_ref`.
      
      Some parts of folly still support legacy versions of C++ but we do not necessarily run the tests on legacy platforms.
      
      Reviewed By: akrieger
      
      Differential Revision: D22635366
      
      fbshipit-source-id: 8961d3465b63875a660d9e83d094ebf0cd64d1df
      67ed8482
    • Mark Santaniello's avatar
      Add clear() API to folly::Arena · e2b1569f
      Mark Santaniello authored
      Summary: Add a `clear()` API to render the Arena like new, except that previously-allocated blocks are preserved to be reused in the future.  (Caveat: we do not reuse "large blocks")
      
      Reviewed By: yfeldblum, davidtgoldblatt
      
      Differential Revision: D22209898
      
      fbshipit-source-id: c2f27d70ee09855df8c8c6fc6d162fca92b9eac8
      e2b1569f
  6. 18 Jul, 2020 4 commits
    • Yedidya Feldblum's avatar
      Mark invoker operator() as maybe-unused · fdcce5ae
      Yedidya Feldblum authored
      Summary:
      [Folly] Mark invoker `operator()` as `[[maybe_unused]]` when defined in macros.
      
      If the macro is used in a `.cpp` but the generated `operator()` is not invoked, the compiler might warn.
      
      This can easily happen in the following situation:
      
      ```
      FOLLY_CREATE_MEMBER_INVOKER(invoke_foo_fn, foo);
      FOLLY_CREATE_MEMBER_INVOKER(invoke_bar_fn, bar);
      
      using invoke_one_fn = std::conditional_t<
          /* cond */,
          invoke_foo_fn,
          invoke_bar_fn>;
      constexpr invoke_one_fn invoke_on;
      
      template <typename T>
      void go(T& t) {
        invoke_one(t);
      }
      ```
      
      In such a situation, either `invoke_foo_fn::operator()` is seen at a call-site or `invoke_bar_fn::operator()` is seen at the call-site, but not both - one of them is unused, and if the right warnings are enabled, will be warned as unused.
      
      Reviewed By: luciang, markisaa
      
      Differential Revision: D22570047
      
      fbshipit-source-id: a4caa7a95ab74ea075cf41c26525935f3d9186c0
      fdcce5ae
    • Andrii Grynenko's avatar
      Make sure EventBase task destructor runs with the correct RequestContext · eeb59fc3
      Andrii Grynenko authored
      Reviewed By: yfeldblum
      
      Differential Revision: D22604684
      
      fbshipit-source-id: 4dedb9c831d0a2137d16bbe0514e00a1c8f77f4a
      eeb59fc3
    • Stanislau Hlebik's avatar
      remediation of S205607 · e914b363
      Stanislau Hlebik authored
      fbshipit-source-id: 798decc90db4f13770e97cdce3c0df7d5421b2a3
      e914b363
    • Stanislau Hlebik's avatar
      remediation of S205607 · 9e997608
      Stanislau Hlebik authored
      fbshipit-source-id: 5113fe0c527595e4227ff827253b7414abbdf7ac
      9e997608
  7. 17 Jul, 2020 2 commits
    • Yedidya Feldblum's avatar
      FOLLY_KEEP for keeping function definitions · 89bd17d8
      Yedidya Feldblum authored
      Summary: [Folly] `FOLLY_KEEP` for keeping function definitions when using function-sections even if the linker would remove them.
      
      Reviewed By: alexshap
      
      Differential Revision: D22575832
      
      fbshipit-source-id: 08410b3c318e361b1f3efcbb8f168091faed86e8
      89bd17d8
    • Zhengxu Chen's avatar
      Add support for adding context string callback. · f50f791e
      Zhengxu Chen authored
      Summary: For program running in different general user contexts, e.g. services, multithreading, it's very useful to extend log prefixes to contain the domain specific context strings in debugging. This diff adds one interface function to push one callback to the logger. The callbacks will be executed when the log message data is constructed, and the strings returned by callbacks will be appended to the tail of log entry prefixes.
      
      Reviewed By: simpkins
      
      Differential Revision: D21291284
      
      fbshipit-source-id: e141bf1c907f6730ea741d0dc06975add93d489c
      f50f791e
  8. 16 Jul, 2020 11 commits
    • Eric Niebler's avatar
      Fix lifetime issue in coro::Materialize with active union member · 8c7b4243
      Eric Niebler authored
      Summary:
      `folly::coro::Materialize` uses a `union` of `ManualLifetime` objects to store either the result (value) or the error of an async operation. However, it accesses these fields without ever activating them by in-place constructing the `ManualLifetime` objects. Likewise, when changing the active field, the `ManualLifetime` objects must by destroyed. It doesn't matter that these operations are no-ops. The must be there to avoid UB, which can manifest at higher optimization levels.
      
      I fix this by providing two functions: `activate` and `deactivate`, for use constructing and destructing values in `ManualLifetime` objects when those objects are fields in a `union`.
      
      Reviewed By: yfeldblum, kirkshoop
      
      Differential Revision: D22527895
      
      fbshipit-source-id: 8cc1c3fae75672387d977dddeff61c82093abb9a
      8c7b4243
    • Lukasz Piatkowski's avatar
      add Mononoke integration tests CI (#26) · 238972c3
      Lukasz Piatkowski authored
      Summary:
      This diff adds a minimal workflow for running integrations tests for Mononoke. Currently only one test is run and it fails.
      
      This also splits the regular Mononoke CI into separate files for Linux and Mac to match the current style in Eden repo.
      There are the "scopeguard::defer" fixes here that somehow escaped the CI tests.
      Some tweaks have been made to "integration_runner_real.py" to make it runnable outside FB context.
      Lastly the change from using "[[ -v ... ]" to "[[ -n "${...:-}" ]]; in "library.sh" was made because the former is not supported by the default Bash version preinstalled on modern MacOS.
      
      Pull Request resolved: https://github.com/facebookexperimental/eden/pull/26
      
      Reviewed By: krallin
      
      Differential Revision: D22541344
      
      Pulled By: lukaspiatkowski
      
      fbshipit-source-id: 5023d147823166a8754be852c29b1e7b0e6d9f5f
      238972c3
    • Kirk Shoop's avatar
      remove pushmi from folly · 0415fc2a
      Kirk Shoop authored
      Summary: remove pushmi from folly
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22531090
      
      fbshipit-source-id: 3e6561f220cc3fd227dec321112ec848c4f17c6f
      0415fc2a
    • Maged Michael's avatar
      hazptr: Add warning for using default inline executor for asynchronous reclamation · 9683c8db
      Maged Michael authored
      Summary:
      Add warning about using the default inline executor for asynchronous reclamation, which may lead to deadlock if there are unintended circular dependencies. The use of inline executor is likely due to not calling folly::enable_hazptr_thread_pool_executor at some earlier point (e.g., due to not calling folly::init()).
      
      Also changed some function names for easy identification and fixed typos.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22536210
      
      fbshipit-source-id: 89e74411a909ec97b03e8ff90f7039ab233c087b
      9683c8db
    • Lukas Piatkowski's avatar
      Back out "fix boost macOS build with new Xcode" (#27) · 6bc436a6
      Lukas Piatkowski authored
      Summary:
      Pull Request resolved: https://github.com/facebookexperimental/eden/pull/27
      
      Pull Request resolved: https://github.com/facebookexperimental/rust-shed/pull/9
      
      Original diffs: D22417488 (https://github.com/facebook/folly/commit/43d80f110b181dd7f72f0f935322a58f9cbb5147), D22528869 (https://github.com/facebook/folly/commit/3930a6ef4437f161c50ac27141cd397eda999fa7)
      
      Reviewed By: markbt
      
      Differential Revision: D22571972
      
      fbshipit-source-id: c6f013565680a757b642dd79e647207fce3351ec
      6bc436a6
    • Matt Galloway's avatar
      folly | Bypass checking for system preadv and pwritev on iOS Simulator platform · 818b7ab5
      Matt Galloway authored
      Summary: Fix building folly sysuio portability when targeting the iOS simulator, because the current Xcode 12 SDK does not contain `preadv` and `pwritev` for the iOS simulator on Catalina, even though it declares it's there, so we get link errors.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22569172
      
      fbshipit-source-id: 870b47063cb37a24364d1de292aba1f966edfb7c
      818b7ab5
    • Brandon Schlinker's avatar
      Construct from existing raw pointer · 104d728c
      Brandon Schlinker authored
      Summary:
      This diff adds a constructor that takes an `AsyncSocket*`, so that lifecycle observers are informed via `move` when a socket is transformed.
      
      If one `AsyncSocket` is constructed from another using the `AsyncSocket(AsyncSocket* oldAsyncSocket)` constructor, then `move(oldSocket, newSocket)` will be triggered for attached observers, allowing the observers to detach from the old socket and attach to the new socket. However, we currently transform between `AsyncSocket` by detaching the fd and event base, and passing them to the new constructor:
      
      ```
      # from FizzAcceptorHandshakeHelper.cpp
      auto evb = transport_->getEventBase();
      auto fd = transport_->getUnderlyingTransport<folly::AsyncSocket>()
                      ->detachNetworkSocket()
                      .toFd();
      transport_.reset();
      sslSocket_ = createSSLSocket(sslContext_, evb, fd);
      ```
      
      When this happens, any `AsyncSocket::LifecycleObserver` that were attached on accept to become separated from the fd/socket that they're attempting to follow. With this change, we can do the following instead (next diff):
      
      ```
       sslSocket_ = createSSLSocket(
            sslContext_, transport_->getUnderlyingTransport<folly::AsyncSocket>());
      transport_.reset();
      ```
      
      Because `createSSLSocket` uses the `AsyncSocket*` constructor, the `move` observer event will be triggered.
      
      Differential Revision: D21614835
      
      fbshipit-source-id: 6c995f879fe41935850247a28ff8af2b33349445
      104d728c
    • Brandon Schlinker's avatar
      Lifecycle observer · 5e4b2f87
      Brandon Schlinker authored
      Summary:
      Adds `AsyncTransport::LifecycleCallback`, an observer that is notified when a transport is closed and destroyed.
      
      Currently only supported for `AsyncSocket` and derived types (e.g., `AsyncSSLSocket`).
      
      `AsyncSocket::LifecycleCallback` is derived from `AsyncTransport::LifecycleCallback` and adds support for additional lifecycle events relevant to `AsyncSocket`.
      
      Details:
      - Can be used by instrumentation that ties its lifetime to that of the transport.
      - Intentionally separate from all existing callbacks that may be added / used by application logic because it is designed to be used by instrumentation that is generic, and thus separate / unaware of application logic.
      - Multiple observer can be registered, but a `folly::small_vector` is used to minimize alloc overhead in the common case of 0 - 2 being registered.
      - The observer implementation is responsible for calling `removeLifecycleObserver` to remove itself if it is destroyed before the socket is destroyed.
      
      Differential Revision: D21613750
      
      fbshipit-source-id: 92bb5de30bc8bab56fa29e62800bf58e47486f1e
      5e4b2f87
    • Brandon Schlinker's avatar
      Update WriteFlags::EOR and add timestamp flags · 17df5922
      Brandon Schlinker authored
      Summary:
      We have traditionally used `WriteFlags::EOR` to communicate that ACK timestamping should occur. This is confusing and mostly for legacy reasons when we had a custom kernel patch that would send ACK timestamps if the EoR flag was set.
      
      Creating proper flags for ACK and SCHED timestamps. I will integrate the logic that uses these flags into `AsyncSocket` in a subsequent diff.
      
      Differential Revision: D22039799
      
      fbshipit-source-id: 23f534365475bb67f91d86657919cce02439f0c8
      17df5922
    • Brandon Schlinker's avatar
      Fix EOR bug, always pass timestamp flags · 9b15aded
      Brandon Schlinker authored
      Summary:
      Socket timestamps (ACK / TX) and EoR tracking currently break for `AsyncSSLSocket` if SSL renegotiation occurs while a timestamped write / EoR write is in progress.
      
      - If EoR tracking is enabled, the EoR flag and any timestamp flags are not included until `AsyncSSLSocket` writes a buffer containing the final byte to the socket. This is to avoid these flags from being set on a partial write of the passed in buffer.
      - If a write occurs while an SSL renegotiation is in progress, OpenSSL will return `SSL_ERROR_WANT_WRITE`. When this happens, we need to call write again, passing back in the same buffer.
      - The current logic for deciding whether to include the EoR and timestamping flags (`eorAwareSSLWrite`) adds the number of bytes pending to the value returned by `AsyncSSLSocket::getRawBytesWritten` to determine the minimum byte offset when the flags should be added.
        - However, when a write fails due to SSL renegotiation, `getRawBytesWritten` may include some of the bytes that were passed in the last call, despite how they have not actually been written to the transport yet. This is because `getRawBytesWritten` is calculated based on the BIO chain length.
        - As a result, the current logic for calculating the offset on which to add the flags overshoots -- it returns an offset that will never be written. This causes the flags to not be added, and timestamps to timeout.
      - This results in one of two things:
        - Timestamp timeouts, where the timestamps are never received
        - If a subsequent write is timestamped, the timestamps from that write may be used instead. This will cause the timestamps to be inflated, and leads to higher TX / ACK times at upper percentiles.
      
      Fix is as follows:
      - Change the logic that determines whether the EoR is included in the buffer to no longer rely on `getRawBytesWritten`. In addition, simplify logic so that it is no longer a separate function and easier to make sense of.
      - Even if EoR tracking is enabled, always write timestamp flags (TX, ACK, etc.) on every write. This reduces the amount of coordination required between different components. The socket error message handler will end up with more cases of timestamps being delivered for earlier bytes than the last body byte, but they already need to deal with that today due to partial writes.
      
      I considered just outright removing support for EoR tracking (EoR was previously used for timestamping) but decided against this as there's still some value in setting the EoR flag for debugging; see notes in code.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D21969420
      
      fbshipit-source-id: db8e7e5fbd70d627f88f2c43199387f5112b5f9e
      9b15aded
    • Orvid King's avatar
      Drop guards on a few builtins under MSVC · 7f1bda25
      Orvid King authored
      Summary:
      MSVC upstream decided not to add these builtins in the end, so drop the guards as these are still needed.
      Fixes: https://github.com/facebook/folly/issues/1412
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22559891
      
      fbshipit-source-id: 7652da1299d8be7fd64a24f9cffd11b721071d68
      7f1bda25
  9. 15 Jul, 2020 1 commit
    • Kirk Shoop's avatar
      fix compile error · eef55493
      Kirk Shoop authored
      Summary: explicitly move exception_ptr arg to fix error
      
      Reviewed By: Orvid
      
      Differential Revision: D22528984
      
      fbshipit-source-id: e72e6008e6a899663970c6dff43c8790d044dd69
      eef55493