- 13 Nov, 2020 5 commits
-
-
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
-
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
-
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
-
Pranjal Raihan authored
Reviewed By: andriigrynenko Differential Revision: D24698220 fbshipit-source-id: aab8c96440d60bb653f0c537af56081819129306
-
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
-
- 12 Nov, 2020 3 commits
-
-
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
-
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
-
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
-
- 11 Nov, 2020 1 commit
-
-
John Reese authored
Reviewed By: zertosh Differential Revision: D24880203 fbshipit-source-id: 2034cdfc2712209e86d3d05c119c58f979b05c52
-
- 10 Nov, 2020 3 commits
-
-
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
-
Michel Salim authored
Summary: This header is needed by python/futures.h Reviewed By: yfeldblum Differential Revision: D24861734 fbshipit-source-id: 913ad13e6ca155250238020538af762c2360f137
-
Dan Melnic authored
Summary: Allow recycling of std::unique_ptr<IOBuf> Reviewed By: yfeldblum Differential Revision: D24650191 fbshipit-source-id: 85d219052cc62c35098085abb3eed6cfe00beefc
-
- 09 Nov, 2020 4 commits
-
-
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
-
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
-
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
-
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
-
- 07 Nov, 2020 2 commits
- 06 Nov, 2020 4 commits
-
-
Koray Polat authored
Reviewed By: rsunkad Differential Revision: D24750170 fbshipit-source-id: 5c48ab812b5438a33713315faf83e7a21a3c4eae
-
TJ Yin authored
Reviewed By: yfeldblum Differential Revision: D24724575 fbshipit-source-id: 2d7c6dbf94ee27638c92abfb94d8084beabb3d5e
-
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
-
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
-
- 05 Nov, 2020 6 commits
-
-
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
-
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
-
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
-
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
-
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
-
Alexey Spiridonov authored
Summary: By putting this in `fizz-config.cmake`, we can use depend on the `sodium` target without compromising our dependents ability to find the library. Put the search module in the common location under `fbcode_builder/CMake` to let dependents use it. Reviewed By: yfeldblum Differential Revision: D24686041 fbshipit-source-id: 942d1ab34feef6cadac2b584eb8cb2d999bae0ca
-
- 04 Nov, 2020 3 commits
-
-
Yedidya Feldblum authored
Summary: [Folly] Use `::new` and `std::addressof` in `Try` to evade custom overloads of `operator new` and of `operator&` in certain types. Reviewed By: lewissbaker Differential Revision: D24698131 fbshipit-source-id: 9b4a3a5e9962018450104ee0d6df2cf58a2881b7
-
Francis Ricci authored
Summary: mmap() calls are a bit dangerous, in that the backing files can be corrupted or unreadable. Ideally, we would just migrate this whole symbolizer to use ::pread(), but in the meantime, just making sure that the last byte of the mmap'd file is readable seems to get rid of the crashes we've been seeing. Reviewed By: ccdavid Differential Revision: D24505608 fbshipit-source-id: e7bb3ea4d34f7b79f5a1cbb593a195f836d85ec7
-
TJ Yin authored
Summary: This diff appended uuid to copied files so that we won't run into race condition when 2 tests are executed at the same time. Reviewed By: meyering Differential Revision: D24589043 fbshipit-source-id: edb6b6eb6c8749694bdad15eb8d00d9f8d931cfc
-
- 02 Nov, 2020 4 commits
-
-
John Kearney authored
Summary: - Fix linker error when building with clang on windows Reviewed By: yfeldblum, jdonald Differential Revision: D24683230 fbshipit-source-id: fe9e5daebb058e007d8f5725ef31e62967a94f15
-
Nicholas Ormrod authored
Summary: The null-deleter test case showcases not_null's handling of null deleters. It also tries to demonstrate that std::unique_ptr does not handle null deleters properly (e.g. by segfaulting). The latter is really "check what kind of undefined behavior we encounter", and this doesn't always work. Surprise! Specifically, the undefined behavior encountered when supplying a null deleter into std::unique_ptr when using tsan is an infinite hang, which breaks the test suite. Fortunately, testing std::unique_ptr's handling of null deleters is irrelevant to the correctness of not_null_unique_ptr, so we can remove this code. After this diff, the following test does not hang: buck test mode/dev-tsan //folly/memory/test:not_null_test -- 'nn\.null_deleter' --run-disabled Reviewed By: yfeldblum Differential Revision: D24662902 fbshipit-source-id: 230187890b29e1d1a5c8284958c9b798c891c7f7
-
Dan Melnic authored
Summary: Fix heap use after free test issue Reviewed By: danobi Differential Revision: D24656212 fbshipit-source-id: 03ab1ee563f56e55a7b793155355c43bdfa11fdf
-
Kyle Nekritz authored
Reviewed By: yfeldblum Differential Revision: D24660041 fbshipit-source-id: fe31b99206d5bf5b643d8e8111b25c570fe7c0d8
-
- 31 Oct, 2020 1 commit
-
-
Aaryaman Sagar authored
Summary: This test inexplicably aborts when ran under TSAN. TSAN reports a read-write race where the exception is freed by the lock-holder / combiner thread and read concurrently by the thread that requested the combine operation. TSAN reports a similar read-write race for this code as well https://gist.github.com/aary/a14ae8d008e1d48a0f2d61582209f217 (copied below). Which is easier to verify as being correct. Though this does not conclusively prove that this test and implementation are race-free, the above repro combined with error-free stress runs of this test under other modes (optimized builds with and without both ASAN and UBSAN) give us a high signal at TSAN's error being a false-negative. So disabling it for now until some point in the future where TSAN stops reporting this as a false-negative ``` namespace { struct Storage { std::atomic<std::uint64_t> futex_{0}; std::atomic<std::uint64_t> next_; std::aligned_storage_t<48, 8> storage_; }; template <typename Waiter> void transferCurrentException(Waiter* waiter) { assert(std::current_exception()); new (&waiter->storage_) std::exception_ptr{std::current_exception()}; waiter->futex_.store(1, std::memory_order_release); } template <template <typename> class Atom = std::atomic> void concurrentExceptionPropagationStress( int, std::chrono::milliseconds) { auto exceptions = std::vector<std::unique_ptr<Storage>>{}; exceptions.resize(1000000); for (auto i = std::size_t{0}; i < exceptions.size(); ++i) { exceptions[i] = std::make_unique<Storage>(); } auto throwing = std::function<void(std::uint64_t)>{[&](auto counter) { if (counter % 2) { throw std::runtime_error{folly::to<std::string>(counter)}; } }}; std::cout << "Started test" << std::endl; auto writer = std::thread{[&]() { for (auto i = std::size_t{0}; i < exceptions.size(); ++i) { try { std::this_thread::yield(); throwing(i); } catch (...) { transferCurrentException(exceptions.at(i).get()); continue; } exceptions.at(i)->futex_.store(2, std::memory_order_release); } }}; auto reader = std::thread{[&]() { for (auto i = std::size_t{0}; i < exceptions.size(); ++i) { auto value = std::uint64_t{}; while (!(value = exceptions.at(i)->futex_.load(std::memory_order_acquire))) {} if (value == 1) { try { auto buf = reinterpret_cast<std::exception_ptr*>(&exceptions.at(i)->storage_); auto ex = folly::launder(buf); auto copy = std::move(*ex); ex->exception_ptr::~exception_ptr(); std::rethrow_exception(std::move(copy)); } catch (std::exception& exc) { assert(folly::to<std::uint64_t>(exc.what()) == i); } } } }}; reader.join(); writer.join(); } } // namespace ``` Reviewed By: yfeldblum Differential Revision: D24653383 fbshipit-source-id: 3ce166766eb42b22ec7b8cfaf8d31ca53580ff9e
-
- 30 Oct, 2020 4 commits
-
-
Lewis Baker authored
Summary: Fixes compilation of `co_await co_awaitTry(someTask.scheduleOn(ex))` when awaited within other `Task` coroutines by adding the `await_resume_try()` method to ViaIfAsyncAwaiter. Also added some unit-tests for this case. Reviewed By: andriigrynenko Differential Revision: D24650827 fbshipit-source-id: 687717d9f73f1fdcfb1f2ff17e4ddca46c16b189
-
Dan Melnic authored
Summary: Fix async socket test (stack-use-after-scope) Reviewed By: andriigrynenko Differential Revision: D24650836 fbshipit-source-id: 795fb71770612a409195d93216c7c5e129bea605
-
Dan Melnic authored
Summary: Avoid dereferencing a nullptr socket_ Reviewed By: danobi Differential Revision: D24652875 fbshipit-source-id: 63a9535226c8ff58fd5348f3c38f5c6c498d9527
-
Matt Joras authored
Summary: It's useful to be able to use the v4-mapped addresses sometimes. Reviewed By: avasylev Differential Revision: D24371807 fbshipit-source-id: 74fe13fa4bef9c77cc51a18137559facda9bdbeb
-