1. 19 Nov, 2020 5 commits
    • Lewis Baker's avatar
      Add async stack-trace support to DetachedInlineTask · beb3e640
      Lewis Baker authored
      Summary:
      The DetachedInlineTask coroutine type now has its own AsyncStackFrame.
      
      While this is largely an internal implementation detail used only
      for the TaskWithExecutor's `.start()` family of methods, this now
      means that detached tasks launched using TaskWithExecutor's
      `.start()` methods will now record the return-address of the call to
      `.start()` in the stack-trace and will also have the async-stack begin
      with the `detached_task` frame at the top-level.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D24440817
      
      fbshipit-source-id: 861f1ca46ec4a28b5b7e388bb8b243e4d597ee0d
      beb3e640
    • Lewis Baker's avatar
      Add async stack support to AsyncGenerator · 155f5afa
      Lewis Baker authored
      Summary:
      AsyncGenerator coroutines now have their own AsyncStackFrame and so can
      participate in async call-stack tracing.
      
      AsyncGenerator now applies co_withAsyncStack() to all awaitables
      awaited from within the generator coroutine.
      
      The NextAwaiter now implements the co_withAsyncStack() CPO to advertise
      that it will save/restore the awaiting coroutine's AsyncStackFrame if
      it provides one.
      
      This allows the AsyncGenerator to symmetric-transfer to awaited Task
      coroutines and vica-versa. This fixes some stack-overflow issues with
      some synchronously-completing use-cases of AsyncGenerator.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D24437278
      
      fbshipit-source-id: f9612d54558b961bb023460d4a118a773022737b
      155f5afa
    • Lewis Baker's avatar
      Add support for async stack-traces to Barrier/BarrierTask · 892c0c31
      Lewis Baker authored
      Summary:
      `BarrierTask` and `DetachedBarrierTask` coroutines now have their own `AsyncStackFrame`.
      
      This means that when a `Task` executes `co_await barrier.arriveAndWait()` that we
      can use symmetric transfer to resume the `Task` since the `BarrierTask` will
      already have an active `AsyncStackRoot` that we can reuse.
      
      This fixes the test for stack-overflow of synchronously completing coroutines that
      use `collectAll()` algorithms (which internally use `BarrierTask`).
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D24435340
      
      fbshipit-source-id: 87b622e8083f6c8f2b12f6b2f4b0bccd8bd5a595
      892c0c31
    • Lewis Baker's avatar
      Add support for async stack traces to folly::coro::Task coroutines · 70852ff3
      Lewis Baker authored
      Summary:
      Initial diff that adds support for tracing through a chain of folly::coro::Task coroutines.
      
      This adds some scaffolding that handles saving/restoring the active AsyncStackFrame
      when a Task awaits some awaitable type that does not know about AsyncStackFrame
      objects but also allows awaitables to opt-in to AsyncStackFrame awareness by
      customising the new `folly::coro::co_withAsyncStack()` CPO.
      
      Currently only `Task` and `TaskWithExecutor` awaiters have customised this CPO.
      
      Also updated the awaiters for `Task` and `TaskWithExecutor` to handle being awaited
      from coroutines that are not async-stack aware - in which case they just record a
      null parent-frame for the Task.
      
      The Task's `final_suspend()` then either deactivates or pops the frame depending
      on whether there was a parent frame recorded.
      
      BUG: This change currently breaks the symmetric-transfer stack-overflow avoidance
      when awaiting synchronously-completing coroutines from an AsyncGenerator or from
      a BarrierTask (eg. inside collectAll implementations).
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D24428736
      
      fbshipit-source-id: 5722e511ad10d95198ae70a5afe567d83cb06285
      70852ff3
    • Chad Austin's avatar
      convert a path to valid glob syntax when prefetching · 2c41d995
      Chad Austin authored
      Summary:
      Paths are not necessarily legal glob syntax. In particular, backslash
      is used for escaping. This caused problems on Windows, where we tried
      to pass a backslash-delimited path into `eden prefetch --no-prefetch`.
      
      Reviewed By: xavierd
      
      Differential Revision: D25072784
      
      fbshipit-source-id: 9ce8e5ccc8f28581512c39d04922889da0bc1bf6
      2c41d995
  2. 17 Nov, 2020 4 commits
  3. 16 Nov, 2020 1 commit
    • Pranjal Raihan's avatar
      Fix folly/io/async/test:async_test - AsyncSocketTest.ConnectionExpiry · ff2ecd36
      Pranjal Raihan authored
      Summary: The worker thread just needed to be killed before the `AcceptCallback`. `AsyncServerSocket` calls a method on this callback class in its dtor, so it must either be alive or removed using `removeAcceptCallback`.
      
      Differential Revision: D24972293
      
      fbshipit-source-id: 2c7b6d64c2b9b8c00d03107002fa31e565df2a01
      ff2ecd36
  4. 13 Nov, 2020 8 commits
    • Pranjal Raihan's avatar
      Add API to AsyncServerSocket that allows potentially stale connections to be dropped · 04eafeb6
      Pranjal Raihan authored
      Summary:
      Added a timestamp to `AsyncServerSocket::QueueMessage` so that `NotificationQueue` can ignore new connection messages which are deemed *expired*. Expired messages represent sockets which have *probably* timed out already.
      
      The TTL is configured per `AsyncServerSocket` instance and is applied to all future messages that are queued by it. By default, messages do not expire. This can be configured with `AsyncServerSocket::setQueueTimeout`.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D24667870
      
      fbshipit-source-id: 0f9d6c235627393d964e280a0d5956676010c7aa
      04eafeb6
    • Pranjal Raihan's avatar
      Allow AtomicNotificationQueue consumers to discard dequeued tasks · d366ea8f
      Pranjal Raihan authored
      Summary:
      If `Consumer::operator()` returns `AtomicNotificationQueueTaskStatus` then this value indicates if the consumer actually consumed the task, or if it should be discarded.
      
      Enumerating all cases:
      
      `operator()` return type is `void`:
      *All tasks are considered consumed and counted towards `maxReadAtOnce`.*
      
      `operator()` returns `AtomicNotificationQueueTaskStatus::CONSUMED`:
      *Same as above.*
      
      `operator()` returns `AtomicNotificationQueueTaskStatus::DISCARD`:
      *Task does not count towards `maxReadAtOnce` and is silently discarded. The consumer is responsible for any cleanup other than calling the destructor.*
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D24698221
      
      fbshipit-source-id: 0da2364d18b67468addf737b67cae573c89b7e9c
      d366ea8f
    • Abdulbaki Aydin's avatar
      Add an API to set signature algorithms in string form · 9f87fdb8
      Abdulbaki Aydin authored
      Summary:
      Add an API to enable setting signature algorithms in string form.
      An example string form: `"ECDSA+SHA256:RSA+SHA256"`.
      OpenSSL supports TLS1.3 Signature Scheme format,
      e.g.; `"ecdsa_secp256r1_sha256:rsa_pss_rsae_sha256"`.
      
      Reviewed By: mingtaoy
      
      Differential Revision: D24745481
      
      fbshipit-source-id: 018be0a242d922f36a81d051791679a20fe08893
      9f87fdb8
    • jmccl's avatar
      Fix build when liburing is installed (#1487) · cc1be4ce
      jmccl authored
      Summary:
      Fixes https://github.com/facebook/folly/issues/1486
      
      Pull Request resolved: https://github.com/facebook/folly/pull/1487
      
      Reviewed By: Orvid
      
      Differential Revision: D24851586
      
      Pulled By: yfeldblum
      
      fbshipit-source-id: ae683ba73336c562a8912ed35ef2b3318ab53519
      cc1be4ce
    • Pratik Shah's avatar
      Subprocess now does not require child processes to be reaped before destruction · 660d9244
      Pratik Shah authored
      Summary: To support cases where the child process may outlive the parent process.  Detaching the process is an option as well, but in that case the pid for the child process is not captured.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24409176
      
      fbshipit-source-id: f74f64e3ae21baa40468c6f39ac6b4d0152db03a
      660d9244
    • Harshit Saokar's avatar
      Instrument time spent by fiber in running state and log it for SR eventbase threads · da355ecd
      Harshit Saokar authored
      Summary:
      We often run into cases where system CPU util is low but SR eventbase threads are backing up. During S213134 debugging, it took us a while to figure out that requests with http transport were expensive and causing issues on intern-rpc clients and later realized it was a known issue for http transport to be CPU intensive. We got lucky that http transport requests used different stack we could see in strobelight, but this will likely not be the case for other similar issues.
      
      For ease of debugging these issues, one idea is to log SR Eventbase cost of each request to SR scuba. **The closest proxy for this is the wall clock time spent by SR's request-dispatch task in fiber, excluding all the preemptions.** This metric does have couple of drawbacks:
      
      - It wont capture all the work a request does via runInMainContext. This should be Ok since SR tries not to do lot of work in mainContext of fiber manager.
      - If system context switching the threads a lot and if eventbase thread is preempted by OS, we will count that time as time spent in task. I am hoping that new metric will still be useful even with this caveat.
      
      Design: Allow fiberManager.AddTask() to optionally take in a bool `logRunningTime` to decide whether it can log the running time of the task. If `logRunningTime` is set, fiber will keep track of elapsed time since most recent run in  `currStartTime_` and track the running time of all previous runs together in  `prevDuration_`.
      
      I looked into using ExecutionObserver for this purpose. I did not go that route because
      1. ExecutionObserver's callbacks return fiber IDs, we want a callback to return function/task ID that we can tie back to Sr request.  See here https://fburl.com/diffusion/qh9ioeq3
      2. Caller (SR) will need to maintain the separate cache of all outstanding tasks with their running_times and manage their lifetime, which feels more complex than current approach.
      
      I have a way to turn off this change using new killswitch I added in SR.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24823396
      
      fbshipit-source-id: 62f79b8c340cc48a22684c9cb3cdddadc260a0ab
      da355ecd
    • Pranjal Raihan's avatar
      Migrate AsyncServerSocket to use AtomicNotificationQueue · ceee59a7
      Pranjal Raihan authored
      Reviewed By: andriigrynenko
      
      Differential Revision: D24698220
      
      fbshipit-source-id: aab8c96440d60bb653f0c537af56081819129306
      ceee59a7
    • Pranjal Raihan's avatar
      Add ability to bound AtomicNotificationQueue size · dcddc5c0
      Pranjal Raihan authored
      Summary:
      Allowing a bounded `AtomicNotificationQueue` is helpful to migrate `AsyncSocketServer` over from `NotificationQueue`.
      
      `AtomicNotificationQueue::tryPutMessage` is the same as `putMessage` but also accepts a `maxSize`. If the queue size is >= `maxSize`, the message will not be queued. The user of the queue is responsible maintaining state regarding the max size of the queue.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D24698219
      
      fbshipit-source-id: 1e3d1dc0073c65cd1803629f6a4a9b015e111b08
      dcddc5c0
  5. 12 Nov, 2020 3 commits
    • Lee Howes's avatar
      Add retryingUnsafe · e5bb59b3
      Lee Howes authored
      Summary: Add retryingUnsafe as a duplicate of retrying that forces the return type to be a Future rather than a SemiFuture.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24707194
      
      fbshipit-source-id: cb2a107f51d4d4dd47a41ce6f8a009c4d82e3138
      e5bb59b3
    • Michel Salim's avatar
      add shared library support to add_fbthrift_cpp_library · cfa6e9c6
      Michel Salim authored
      Summary:
      `add_fbthrift_cpp_library` should honor `BUILD_SHARED_LIBS`, and call
      `add_library` with the right setting (`SHARED` if enabled, `STATIC` otherwise)
      
      Reviewed By: yns88
      
      Differential Revision: D24911124
      
      fbshipit-source-id: 79df7640a758a592a3df3e9e79bb129dd57f2d47
      cfa6e9c6
    • Luca Niccolini's avatar
      add zlib as an explicit dependency for getdeps build · d4340c0a
      Luca Niccolini authored
      Summary:
      needed for QUIC/H3 interop
      
      also make it possible to run getdeps build with extra arguments (--no-tests for
      example)
      
      Reviewed By: mjoras
      
      Differential Revision: D24925777
      
      fbshipit-source-id: fbdc1aa56e398d295ef8dac0ad0bab03bd7bd803
      d4340c0a
  6. 11 Nov, 2020 1 commit
  7. 10 Nov, 2020 3 commits
    • Davide Cavalca's avatar
      proxygen: fix shared libs build · bfe27697
      Davide Cavalca authored
      Summary:
      Right now proxygen hardcodes a static build when using cmake  and
      ignores BUILD_SHARED_LIBS. Fix that, and enable PIE on the shared libs so they
      can be linked properly
      
      Closes: https://github.com/facebook/proxygen/issues/335
      
      Reviewed By: mjoras, lnicco
      
      Differential Revision: D24787944
      
      fbshipit-source-id: 7a654af7cb43227ca913a1bed67f2432703a343d
      bfe27697
    • Michel Salim's avatar
      also install executor.h · 6cd0c17d
      Michel Salim authored
      Summary: This header is needed by python/futures.h
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24861734
      
      fbshipit-source-id: 913ad13e6ca155250238020538af762c2360f137
      6cd0c17d
    • Dan Melnic's avatar
      Allow recycling of std::unique_ptr<IOBuf> · d3489f9e
      Dan Melnic authored
      Summary: Allow recycling of std::unique_ptr<IOBuf>
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24650191
      
      fbshipit-source-id: 85d219052cc62c35098085abb3eed6cfe00beefc
      d3489f9e
  8. 09 Nov, 2020 4 commits
    • Michel Salim's avatar
      fix Python binding installation · 4087512f
      Michel Salim authored
      Summary:
      Honor `DESTDIR` on Unix-like platforms (important when building packages),
      and use `CMAKE_INSTALL_PREFIX` instead of the default prefix.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24821935
      
      fbshipit-source-id: 42a311f920ed74760597f8908cd339b914ecb92e
      4087512f
    • Eric Niebler's avatar
      exception_wrapper::handle should accommodate noexcept lambdas in C++17 · 477a4c23
      Eric Niebler authored
      Summary: `exception_wrapper::handle` uses the signature of `Fn::operator()` to pick apart the function type and deduce the exception type for the handler. We use specializations of an `arg_type_` class template to do this. In C++17, `noexcept` is a part of the type system, but there were no specializations of `arg_type_` to handle `noexcept`-qualified callables.
      
      Reviewed By: ispeters
      
      Differential Revision: D24733871
      
      fbshipit-source-id: 344e735e653296c9344b6c09662fccd94953e8a7
      477a4c23
    • Daan De Meyer's avatar
      Add Decider argument to retryN and retryWithExponentialBackoff · 11440055
      Daan De Meyer authored
      Summary:
      This allows customizing the exceptions for which retry
      is triggered. This is useful when we only want to retry
      in case of a few specific exceptions but have others
      cause the operation to fail immediately.
      
      Reviewed By: lewissbaker
      
      Differential Revision: D24766615
      
      fbshipit-source-id: f97451673f575bef511399cbde6c1ad110f9493a
      11440055
    • Laurent Stacul's avatar
      Fix missing #include <limits> (#1482) · 9939b376
      Laurent Stacul authored
      Summary:
      Hello,
      The compilation with the HEAD of gcc 11 fails:
      ```
      FAILED: CMakeFiles/folly_base.dir/folly/TimeoutQueue.cpp.o
      g++  ... -c ../folly/TimeoutQueue.cpp
      ../folly/TimeoutQueue.cpp: In member function 'int64_t folly::TimeoutQueue::nextExpiration() const':
      ../folly/TimeoutQueue.cpp:39:32: error: 'numeric_limits' is not a member of 'std'
         39 |       timeouts_.empty() ? std::numeric_limits<int64_t>::max()
            |                                ^~~~~~~~~~~~~~
      ../folly/TimeoutQueue.cpp:39:54: error: expected primary-expression before '>' token
         39 |       timeouts_.empty() ? std::numeric_limits<int64_t>::max()
            |                                                      ^
      ../folly/TimeoutQueue.cpp:39:57: error: '::max' has not been declared
         39 |       timeouts_.empty() ? std::numeric_limits<int64_t>::max()
            |                                                         ^~~
      ../folly/TimeoutQueue.cpp:39:57: note: suggested alternatives:
      In file included from /opt/1A/toolchain/x86_64-v21.0.8/include/c++/11.0.0/functional:65,
                       from ../folly/TimeoutQueue.h:31,
                       from ../folly/TimeoutQueue.cpp:17:
      /opt/1A/toolchain/x86_64-v21.0.8/include/c++/11.0.0/bits/stl_algo.h:3464:5: note:   'std::max'
       3464 |     max(initializer_list<_Tp> __l, _Compare __comp)
            |     ^~~
      ```
      It is nothing to fix.
      Regards,
      Laurent
      
      Pull Request resolved: https://github.com/facebook/folly/pull/1482
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24757105
      
      Pulled By: Orvid
      
      fbshipit-source-id: 8a5382edbe5b7c11cace5a1cd4c0645dfd2a9d37
      9939b376
  9. 07 Nov, 2020 2 commits
  10. 06 Nov, 2020 4 commits
    • Koray Polat's avatar
      Add an option to specify lfs path · 7ddffe80
      Koray Polat authored
      Reviewed By: rsunkad
      
      Differential Revision: D24750170
      
      fbshipit-source-id: 5c48ab812b5438a33713315faf83e7a21a3c4eae
      7ddffe80
    • TJ Yin's avatar
      fix tsan unit-test failure by reducing iteration · a6098259
      TJ Yin authored
      Reviewed By: yfeldblum
      
      Differential Revision: D24724575
      
      fbshipit-source-id: 2d7c6dbf94ee27638c92abfb94d8084beabb3d5e
      a6098259
    • Yi Zhang's avatar
      Record owner gettid() if TrackThreadId is enabled · 1540c39c
      Yi Zhang authored
      Summary:
      Adds a new TrackThreadId template argument to SharedMutexImpl that when enabled, expands SharedMutex from 4 to 8 bytes (still with
      4 byte alignment), and uses the extra space to record the thread ID
      of a distinguished owning thread (upgrade or exclusive lock holder).
      This dramatically simplifies debugging in some scenarios.  It adds enough
      information that we could enforce that unlock happens on the same thread
      as lock, but this diff doesn't actually add such a check.
      
      A new TrackedSharedMutex class is added for SharedMutex with lock owner thread id tracking enabled. fb_localtime is the only user for now to reduce the risk.
      
      Differential Revision: D24731513
      
      fbshipit-source-id: 6b41fb05498842224feb088b1332794281b501a9
      1540c39c
    • Lee Howes's avatar
      Move retrying parameter to match old behaviour · dd4c59a9
      Lee Howes authored
      Summary: The old behaviour passed an r-value after incrementing. Some use cases take an r-value and hence depend on that behaviour. This change makes that consistent.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24634859
      
      fbshipit-source-id: 872197a11197a5e60f32492f1670dd94cf78c8b8
      dd4c59a9
  11. 05 Nov, 2020 5 commits
    • Lukas Piatkowski's avatar
      rust-shed/futures_01_ext: rename futures_ext to futures_01_ext · b519e71b
      Lukas Piatkowski authored
      Summary: As part of the effort to deprecate futures 0.1 in favor of 0.3 I want to create a new futures_ext crate that will contain some of the extensions that are applicable from the futures_01_ext. But first I need to reclame this crate name by renaming the old futures_ext crate. This will also make it easier to track which parts of codebase still use the old futures.
      
      Reviewed By: farnz
      
      Differential Revision: D24725776
      
      fbshipit-source-id: 3574d2a0790f8212f6fad4106655cd41836ff74d
      b519e71b
    • Wez Furlong's avatar
      getdeps: don't depend on git fetch depth any longer · 079cd3cc
      Wez Furlong authored
      Summary:
      This commit takes advantage of git 2.5.0 being able to fetch a
      requested revision rather than relying on the desired revision being within the
      depth limited fetch.
      
      This relies on having git 2.5.0 on the server which is true for all
      of the projects we have manifests for; this shows zero matches:
      
      ```
      $ rg repo_url opensource/fbcode_builder/manifests | grep -v github
      ```
      
      We've had a couple of situations recently where folks have run into issues with
      the commit rate in folly being higher than then fetch depth, so this should
      address that.
      
      Refs: https://github.com/facebook/watchman/issues/866
      
      Reviewed By: fanzeyi
      
      Differential Revision: D24747992
      
      fbshipit-source-id: e9b67c61dddc9f55e05d8984e8d210e7d2faabcb
      079cd3cc
    • Andrii Grynenko's avatar
      Make coro::sleep throw when cancelled · 8dfce3ec
      Andrii Grynenko authored
      Summary: We shouldn't be sleeping less that requested and return silently.
      
      Reviewed By: lewissbaker
      
      Differential Revision: D24633977
      
      fbshipit-source-id: 71ef422f0f72747bc19ac8491fdb8b148598d499
      8dfce3ec
    • Junqi Wang's avatar
      Fix tsan warning in AsyncUDPSocket test · 244b6f44
      Junqi Wang authored
      Summary:
      tsan reports warning in the test because the server socket writes in
      a thread but closes in another thread without synchronization. This was by
      design and is not a bug since implicit synchronization is done by joining client
      thread, but this isn't visible to tsan. The fix is to use a port reused new
      server socket to write response to client.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24675113
      
      fbshipit-source-id: bdab8a5d5b6f85aa36ba84a6299e4457710625de
      244b6f44
    • Alexey Spiridonov's avatar
      Fix OSS build · aaf2076d
      Alexey Spiridonov authored
      Summary:
      There are two separate changes here.
      
      ### Use `find_package`
      
      The old setup of "let's manually enumerate and order the libraries that Bistro depends on" worked fine, except:
       - it was a bit brittle (requiring occasional patches as deps changed), and
       - it garnered a lot of feedback to the effect of "your build is weird, so it's probably broken because of that."
      
      Now I expect to have fewer breaks and more plausible deniability :)
      
       More importantly, this should make it much easier to migrate to `getdeps.py`.
      
      ## Statically link `fmt`
      
      After `fmt` was added as a `folly` dependency, and linked into Folly code used by Bistro, its tests would fail to run with this error: `test_sqlite_task_store: error while loading shared libraries: libfmt.so.6: cannot open shared object file: No such file or directory`.
      
      Something was getting messed up in the dynamic linking, and it wasn't clear to me what -- the way that Bistro is linking its dependencies certainly seems sensible. Most likely one of the dependencies is incompatible with dynamic linking in a subtle way. I suspect Proxygen.
      
      The `fmt.py` change in this diff addresses this problem by forcing static linking on the offending library.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D24604309
      
      fbshipit-source-id: 35ecbbb277b25907ecaee493e8b0081d9f20b865
      aaf2076d