- 02 Mar, 2021 1 commit
-
-
Yingnan Li authored
Use ExecutionObserver in EventBase to monitor function not executed EventBaseAtomicNotificationQueueby in EventBase Summary: Our service schedules tasks (eSemiFuture/coroutine) on SerialExecutor on top of IOThreadPoolExecutor. When using EventBaseWatchdog to locate long running tasks, I found some long running tasks were not observed by PerThreadExecController. After some investigation, I found there are 2 cases in EventBase that tasks are not executed by handlerReady in EventBaseAtomicNotificationQueue. 1. Short-circuit scenario in runInEventBaseThread which bypasses EventBaseAtomicNotificationQueue. 2. If notification Queue is marked 'internal', some events may be ran "manually" in loopBody. As a result, the observer logic in EventHandler (EventBaseAtomicNotificationQueue derives from EventHandler) is passed. Reviewed By: andriigrynenko Differential Revision: D26563459 fbshipit-source-id: e2741cff373c89a5cd438904f910b23a64fad653
-
- 01 Mar, 2021 4 commits
-
-
Brandon Schlinker authored
Summary: Adding support for write and socket timestamps by introducing `ByteEvent` that can be delivered to observers. `AsyncTransport::WriteFlags` has long had timestamping related flags, such as `TIMESTAMP_TX`, but the code required to act on these flags only existed in proxygen. This diff generalizes the approach so that it works for other use cases of `AsyncSocket`. This diff is long, but much of it is unit tests designed to prevent regressions given the trickiness of socket timestamps and `ByteEvent`. **Each `ByteEvent` contains:** - Type (WRITE, SCHED, TX, ACK) - Byte stream offset that the timestamp is for (relative to the raw byte stream, which means after SSL in the case of AsyncSSLSocket) - `steady_clock` timestamp recorded by AsyncSocket when generating the `ByteEvent` - For SCHED, TX, and ACK events, if available, hardware and software (kernel) timestamps **How `ByteEvent` are used:** - Support is enabled when an observer is attached with the `byteEvents` config flag set. If the socket does not support timestamps, the observer is notified through the `byteEventsUnavailable` callback. Otherwise, `byteEventsEnabled` is called - When the application writes to a socket with `ByteEvent` support enabled and a relevant `WriteFlag`, SCHED/TX/ACK `ByteEvent` are requested from the kernel, and WRITE `ByteEvent` are generated by the socket for the *last byte* in the write. - If the entire write buffer cannot be written at once, then additional `ByteEvent` will also be generated for the last byte in each write. - This means that if the application wants to timestamp a specific byte, it must break up the write buffer before handing it to `AsyncSocket` such that the byte to timestamp is the last byte in the write buffer. - When socket timestamps arrive from the kernel via the socket error queue, they are transformed into `ByteEvent` and passed to observers **Caveats:** 1. Socket timestamps received from the kernel contain the byte's offset in the write stream. This counter is a `uint32_t`, and thus rolls over every ~4GB. When transforming raw timestamp into `ByteEvent`, we correct for this and transform the raw offset into an offset relative to the raw byte offset stored by `AsyncSocket` (returned via `getRawBytesWritten()`). 2. At the moment, a read callback must be installed to receive socket timestamps due to epoll's behavior. I will correct this with a patch to epoll, see https://github.com/libevent/libevent/issues/1038#issuecomment-703315425 for details 3. If a msghdr's ancillary data contains a timestamping flag (such as `SOF_TIMESTAMPING_TX_SOFTWARE`), the kernel will enqueue a socket error message containing the byte offset of the write ( `SO_EE_ORIGIN_TIMESTAMPING`) even if timestamping has not been enabled by an associated call to `setsockopt`. This creates a problem: 1. If an application was to use a timestamp `WriteFlags` such as `TIMESTAMP_TX` without enabling timestamping, and if `AsyncSocket` transformed such `WriteFlags` to ancillary data by default, it could create a situation where epoll continues to return `EV_READ` (due to items in the socket error queue), but `AsyncSocket` would not fetch anything from the socket error queue. 2. To prevent this scenario, `WriteFlags` related to timestamping are not translated into msghdr ancillary data unless timestamping is enabled. This required adding a boolean to `getAncillaryData` and `getAncillaryDataSize`. Differential Revision: D24094832 fbshipit-source-id: e3bec730ddd1fc1696023d8c982ae02ab9b5fb7d
-
Brandon Schlinker authored
Summary: Wrapper around `folly::netops` methods that makes it easier to mock these methods in unit tests. When we want to mock out calls to `folly::netops` we currently: - Use methods like `getSockOptVirtual` and `setSockOptVirtual` - Mock part of the socket, like in the tests in `AsyncSSLSocketWriteTest` I've the latter makes the tests particularly error prone, since we're mocking the object that's also under test. This change introduces `netops::Dispatcher`, which is a class containing all of the functions in `folly::netops`: - By default `AsyncSocket` uses a default, static instance of `Dispatcher` that forwards calls to the original `netops::` calls (e.g., calling `netops::Dispatcher::sendmsg` results in a call to `netops::sendmsg`. - When a test wants to mock a a `folly::netops` call, it can call `setOverrideNetOpsDispatcher` to insert a mock `netops::Dispatcher`. I use it in this manner in D24094832 Differential Revision: D24661160 fbshipit-source-id: e9cb4ed28ffe409c74998a1c9501c0706fc853e0
-
Brandon Schlinker authored
Summary: During the wangle accept process and in a few pieces of application code we transform one type of `AsyncSocket` to another, potentially *after* a write has occurred. Since we do not currently carry `appBytesWritten` and `rawBytesWritten` during transformations, those values may not represent all bytes written. Likewise, if we transform from a `AsyncSSLSocket`, we lose count of the number of raw bytes written. It's very difficult to reason about whether these problems will actually manifest, so I'd prefer to just guard against it. With this change, we explicitly track the number of bytes written to the socket by incrementing a counter in `sendSocketMessage`, which is used by both `AsyncSocket` and `AsyncSSLSocket`. In addition, we copy the appBytesWritten and rawBytesWritten during socket moves. Reviewed By: yfeldblum Differential Revision: D24551958 fbshipit-source-id: 88416114b52931ff3ceef847401d556ccf0ab664
-
Brandon Schlinker authored
Summary: Both `AsyncSocket` and `AsyncSSLSocket` currently have code to generate a socket message and control messages with ancillary data. Merge this code into a new function in `AsyncSocket`. Differential Revision: D24096351 fbshipit-source-id: 87d90648c10c87832f868e322acf59c97b8ac8b7
-
- 28 Feb, 2021 1 commit
-
-
Yedidya Feldblum authored
Summary: Move `ready_awaitable`, `variant_awaitable` types to `folly/experimental/coro/Coroutine.h`, which reexports the foundational vocabulary coroutine helper types and is a suitable place for these vocabulary coroutine helper types. `ready_awaitable` is an awaitable which is ready and has a value or, if of void, which is just ready. `variant_awaitable` is an awaitable composed of one of several possible backing awaitables. Reviewed By: aary Differential Revision: D26270528 fbshipit-source-id: e98f64b3d3ffaf5bbd322397a41bb5fe85a6bb55
-
- 27 Feb, 2021 6 commits
-
-
Victor Zverovich authored
Summary: `folly::format` is a problematic API because it returns a `Formatter` object that may store references to arguments as its comment warns: ``` * Formatter class. * * Note that this class is tricky, as it keeps *references* to its lvalue * arguments (while it takes ownership of the temporaries), and it doesn't * copy the passed-in format string. Thankfully, you can't use this * directly, you have to use format(...) below. ``` This has negative safety and performance (encourages reuse of the same formatter) implications because contrary to what the comment says you can use the object directly since it's returned from `folly::format`. For example ``` auto f = folly::format(std::string("{}"), 42); f.str(); ``` is a UB with no compile-time diagnostic (only caught by ASAN). It's also unnecessary because the `Formatter` object is usually converted to string straight away via `str()` or written to an output stream. Reusing the formatter doesn't make much sense either because the expensive part is formatting, not capturing arguments. This diff deprecates `folly::format` suggesting `fmt::format` as a potential replacement. `fmt::format` doesn't have the above problem because arguments are always in scope. It also has the following advantages: * Better compile times. * Compile-time format string checks. * Compatibility with C++20 `std::format`. * Better performance, particularly with format string compilation which eliminates format string processing overhead altogether. * Smaller binary footprint. Also remove `folly::writeTo` which is no longer used and the `format_nested_fbstrings` benchmark which is identical to `format_nested_strings` since there is no `fbstr()` any more. Reviewed By: yfeldblum Differential Revision: D26391489 fbshipit-source-id: f0309e78db0eb6d8c22b426d4cc333a17c53f73e
-
Yedidya Feldblum authored
Summary: Follow a convention that function object types with corresponding function object instances be named with a `_fn` suffix. Reviewed By: ericniebler Differential Revision: D26674781 fbshipit-source-id: f8a3b8f18677c905897da8bae38cc76693146073
-
Yedidya Feldblum authored
Summary: Use of `std::coroutine_traits` and `std::experimental::coroutine_traits` can get awkward and we do not actually need the flexibility. Just define member type aliases `promise_type`. Reviewed By: andriigrynenko Differential Revision: D26706363 fbshipit-source-id: 0cdb59d542a64ff85d8a126003a014b7b6be3134
-
Yedidya Feldblum authored
Summary: Invokers for `begin` and `end` with `std::begin` and `std::end` in scope. Reviewed By: ericniebler Differential Revision: D26683890 fbshipit-source-id: 79e57b235c847b5bea29e2e6aac767bdfcef72e8
-
Yedidya Feldblum authored
Summary: Namely, let the backports of `std::size`, `std::empty`, and `std::data` be function objects in folly to eliminate possibility of ADL overloading. Reviewed By: ericniebler Differential Revision: D26681127 fbshipit-source-id: 0c2868f25818afca7efd6b2337cb398daa9bb7f3
-
Dan Melnic authored
Summary: Add FOLLY_PORT_WIN32_OPEN_BINARY define Reviewed By: yfeldblum Differential Revision: D26693403 fbshipit-source-id: 51fb9ceef31cffdeff691ad0eb49939725cc047b
-
- 26 Feb, 2021 8 commits
-
-
Yedidya Feldblum authored
Summary: Reexport remaining names, including `noop_coroutine`, from `folly/experimental/coro/Coroutine.h`, which wraps inclusion of `experimental/coroutine`. Differential Revision: D26227580 fbshipit-source-id: b7ea0bb0bb7447bf4327b2f178c51db544f70f46
-
Yedidya Feldblum authored
Summary: TSIA Differential Revision: D26697902 fbshipit-source-id: bd0a552959474c0efa6167e8da1ed31916c77346
-
Alfred Fuller authored
Summary: The static cast and forward were backwards. Previously it happily converted an rvalue to a const lvalue. Now it causes the exact same static_assert failure as std::forward in every rvalue->lvalue case. Reviewed By: yfeldblum Differential Revision: D26680263 fbshipit-source-id: f1ca4c4bfdd6333d24fe3c406d5373a2f72dea31
-
Yedidya Feldblum authored
Summary: There are no correct uses of the catch-all overload of `folly::exceptionStr`. Every use is a bug of some kind, leading to the impression that this overload ought to be removed. Differential Revision: D26539622 fbshipit-source-id: dc2ca0781ea02f1327a334bb1fe2e533fa46d1b3
-
Andrew Huang authored
Summary: Prevent a race condition where a session's reference count can be decremented to 0 (causing the session to be freed) by one thread before another thread can increment the reference count. Reviewed By: mingtaoy Differential Revision: D26614966 fbshipit-source-id: 6c55321becfc1a3467f57c692065680a32ac21f3
-
Scott Pruett authored
Summary: BoundedAsyncPipe::write() accepts arguments by-reference, so even these constants may be out-of-scope by the time the coroutine actually runs, causing use-after-scope problems which are detected by ASAN. Wrapping in co_invoke allows us to scope the parameter so that it lives long enough. Reviewed By: lxfind Differential Revision: D26649913 fbshipit-source-id: 5bc6b7f64a5e75c9386b245fa3fd1484efaf1d13
-
Kenny Yu authored
Summary: This is needed if we have an AsyncStackRoot pointer, and we want to call both `getTopFrame()` and `getNextRoot()` (e.g. walking async stacks) without const-casting the pointer. Reviewed By: yfeldblum Differential Revision: D26675131 fbshipit-source-id: 84de25c67d563e118483d67d0fedb4ea7aef2604
-
Yedidya Feldblum authored
Summary: Currently there is an extension point `folly_co_invoke` for providing customizations for `co_invoke`. But `tag_invoke` is intended as a generic mechanism for providing namespaced customizations. So let us use that. Reviewed By: ericniebler Differential Revision: D26674580 fbshipit-source-id: b1baea04996f162699510c3aba8d2c330fcf34bb
-
- 25 Feb, 2021 3 commits
-
-
generatedunixname89002005325676 authored
Reviewed By: zertosh Differential Revision: D26660179 fbshipit-source-id: bc0ae7deeb3a3097bfb77cbf0bc471157f586326
-
Nikita Lutsenko authored
Summary: As per title, stub it out in a way that will just trigger exceptions for everything not yet implemented. Reviewed By: yfeldblum Differential Revision: D26579892 fbshipit-source-id: 5bfc20b1f6737bbda80616ea1407a5cf108be3c3
-
Yedidya Feldblum authored
Summary: Choose a single strategy for guarding coro files. Namely, all includes are unconditional, but otherwise the bodies of all headers, sources, and tests are conditional. Reviewed By: ericniebler Differential Revision: D26647625 fbshipit-source-id: 33eeb12fb231bd7ae7fe5cb97ead16ab017b15f5
-
- 24 Feb, 2021 5 commits
-
-
Eric Niebler authored
Summary: There is a Most Vexing Parse problem in `folly/MapUtil.h`: ``` xplat/folly/MapUtil.h:44:75: error: expected expression return (pos != map.end()) ? (pos->second) : M(std::forward<Value>(dflt)); ^ ``` Fix the issue by replacing paren-init with `static_cast`. Reviewed By: yfeldblum, ispeters, ot Differential Revision: D26610359 fbshipit-source-id: 89842b9b68cebaaab43ea5f82abe7e500f98ff2c
-
Pádraig Brady authored
Summary: Newer libiberty has limits that over constrain the size of symbols that can be demangled. This applies the same adjustment in the default platform009 demangler, done in D26380825 Differential Revision: D26617797 fbshipit-source-id: 8d3e72a0807aa9ecff18d26f38c1acb83f9c051b
-
Ajanthan Asogamoorthy authored
Summary: Right now we have a single option verifyPeer_(SSLVerifyPeerEnum) that controls how verification is done regardless of how the ssl context is inevitably used (client or server). This is confusing for our users since it is 1. unclear of what the current values mean 2. It is unclear that setting an option such as VERIFY_REQ_CLIENT_CERT also sets your verification mode when running as a client.In order to make this api more clear for our users we separate out this enum to two separate enums for client and server. When this is passed to openSSL we still pass the same single mode int. Reviewed By: mingtaoy Differential Revision: D18308192 fbshipit-source-id: 5d49d3dc5117075a55681cf67b5f8e72b5e62c89
-
Vitalii Topoliuk authored
Summary: Original commit changeset: b48fe4683165 Our application can't terminate since this change was committed. 100% reproducible. Reviewed By: WillerZ Differential Revision: D26621716 fbshipit-source-id: 571ecd9a6f5286469877bf510f1f55b036df9217
-
Yair Gottdenker authored
Summary: This is the second attempt, the first one was D22958650. Decided to do a different diff as some affected files were moved from experimental/afrind/coro/h2proxy to proxygen/facebook/lib/experimental/coro/ which created some confusion while arc pulling Reviewed By: yfeldblum Differential Revision: D25432869 fbshipit-source-id: a183898302a79084d890548b9b7ecc4409f501d2
-
- 23 Feb, 2021 10 commits
-
-
Andrew Gallagher authored
Summary: `/proc/self/exe` doesn't exist on apple platforms, so provide an implementation that does. Reviewed By: yfeldblum Differential Revision: D26599826 fbshipit-source-id: e00dde59d639063e59cb97feb75caf0a49a01954
-
Yedidya Feldblum authored
Summary: Move `order_preserving_reinsertion_view` to a new header `folly/container/View.h` and revise its customization facility in terms of `folly::tag_invoke`. Reviewed By: iahs Differential Revision: D25787117 fbshipit-source-id: b7ccd3b6a1413a2639ef1beb1f1ee23bbed045e3
-
Udip Pant authored
Summary: This'll move getdeps dependency for katran to 0.3 (which is already the case for travis build) Reviewed By: avasylev Differential Revision: D26129525 fbshipit-source-id: ec9d0615a3e02d75454a3f9dd974ac5c7589e40a
-
Dan Melnic authored
Summary: IoUringBackend benchmarks Reviewed By: kevin-vigor Differential Revision: D26594012 fbshipit-source-id: c6a08e6ec13b736c32619958713706118e2f1a49
-
Yedidya Feldblum authored
Summary: No need to overload coroutine `promise_type::return_value` to handle initializer list expressions - just template and default the parameter. Reviewed By: Orvid Differential Revision: D26508901 fbshipit-source-id: 61c4a2225355b0044638cd89b9153d599317a453
-
Misha Shneerson authored
Summary: When EventBase is destructed, the underlying local storage should be destructed prior to event loop being destroyed. Reviewed By: andriigrynenko Differential Revision: D26583298 fbshipit-source-id: 9bfb8442b2a827aa2b3564b27ea765ec1e7b26a2
-
Orvid King authored
Summary: Owing to a set of bugs, it is not currently possible to actually declare ranges::enable_view without actually defining it, so we have to include the actual definition when we're in compilation units with it available. If the definition is not available, then the `const` form is sufficient to keep things compiling. Reviewed By: yfeldblum Differential Revision: D26337214 fbshipit-source-id: fb042b62967e3e6f536e8268e1b6dd6ccd20b810
-
Yedidya Feldblum authored
Summary: Some compilers fail when `friend` declarations have mismatched `constexpr`-specifiers. Reviewed By: Mizuchi Differential Revision: D26601032 fbshipit-source-id: a49e327ff9353ddb21eac96776f97d4089c85374
-
Yedidya Feldblum authored
Summary: Use `std::filesystem` or `std::experimental::filesystem`, via `folly/portability/Filesystem.h`, in `folly/test/FileTest.cpp`. Replaces the use of `boost::filesystem`. Reviewed By: Orvid Differential Revision: D26572916 fbshipit-source-id: f769e00b2772f3bfaed899f5694d89db87089e35
-
Yedidya Feldblum authored
Summary: Make the implementation of `folly::fs::lexically_normal` match C++17 `std::filesystem::path::lexically_normal`. The implementation from `boost::filesystem::path::lexically_normal` is easy to fall back on but has some differences. The implementation is copied mutatis mutandis from https://github.com/gulrak/filesystem/ at tag v1.5.0, in which `ghc::filesystem` is a complete portable standalone implementation of the `std::filesystem` spec and released under the MIT license. Reviewed By: Orvid Differential Revision: D26572520 fbshipit-source-id: 5ce03f37c078ee4f62e3480d4abc259199867c8e
-
- 22 Feb, 2021 2 commits
-
-
Yedidya Feldblum authored
Summary: [Folly] Prefer to nest helper functions in coro tests within the specific tests which use them. Reviewed By: ispeters, Orvid Differential Revision: D20324066 fbshipit-source-id: 4134145c406f2d1fdeaddf9a7d244c1bd25ec60b
-
Yedidya Feldblum authored
Summary: Some versions of gcc miscompile the code in the parallel-map exception test. Work around the miscompile by adding an extra log line. Reviewed By: Orvid Differential Revision: D26509079 fbshipit-source-id: 1474437d96aeabad0294bcb395b7baec229eb9ba
-