- 28 Aug, 2018 9 commits
-
-
Matthieu Martin authored
Summary: Save python import cost in folly/python get_fiber_manager. In my testing, using call_once here is safe even in multi-threaded and forked environment. Though I wouldn't be surprised if a corner case is found in the future, as I couldn't find a definite answer about the safety of memoization Python C-API import calls. Reviewed By: fried Differential Revision: D9542909 fbshipit-source-id: c62a08ebf67bd4eb82f1bf4b7657c4a3a6daf4db
-
Aaryaman Sagar authored
Summary: The current futex API required a reference to a futex object in order to invoke `futexWake()`, although this is not buggy by itself, it is technically UB and nothing is stopping ASAN from catching this sort of use-after-free and reporting it as an error. Especially when the futex is represented as a pointer, requiring a dereference to reach a member function The bug can come up when you call `futexWake()` on a futex that has been destroyed, for example ``` auto&& futex_ptr = std::atomic<Futex<>*>{nullptr}; auto&& thread = std::thread{[&]() { auto&& futex = Futex<>{0}; futex_ptr.store(&futex); while (futex.load(std::memory_order_relaxed) != 1) { futex.futexWait(0); } }}; while (!futex_ptr.load()) {} futex_ptr.load()->store(1); futex_ptr.load()->futexWake(1); thread.join(); ``` Here immediately after the `store(1)`, our thread could have loaded the value, seen that it had changed, and never went to sleep. Or it could have seen the value as 0, went to sleep and immediately returned when it saw that the value in the futex word was not what was expected. In the scenario described above calling `futexWake()` is done on a "dangling" pointer. To avoid this, we just never dereference the pointer, and pass the pointer to the futex syscall, where it will do the right things A side benefit to the refactor is that adding specializations is very easy. And we don't have to mess with member function specializations anymore, which are inherently hard to work with (eg. cannot partially specialize) The ADL extension points (currently implemented for `Futex<std::atomic>`, `Futex<DeterministicAtomic>` and `Futex<EmulatedFutexAtomic>`) are ``` int futexWakeImpl(FutexType* futex, int count, uint32_t wakeMask); FutexResult futexWaitImpl( FutexType* futex, uint32_t expected, std::chrono::system_clock::time_point const* absSystemTime, std::chrono::steady_clock::time_point const* absSteadyTime, uint32_t waitMask); ``` Reviewed By: yfeldblum Differential Revision: D9376527 fbshipit-source-id: bb2b54e511fdf1da992c630a9bc7dc37f76da641
-
Nick Terrell authored
Summary: Add the BZIP2 stream codec. The `FlushOp::FLUSH` does not guarantee that the decompressor can read all the input processed so far, due to a bug in the bzip2 library. This is likely not important, since `FLUSH` is not a common operation, especially with bzip2. Reviewed By: yfeldblum Differential Revision: D9484325 fbshipit-source-id: 40770b6f301a16d86c4de8c2b0875f931f00cba2
-
Xiao Shi authored
Summary: F14 library relies on the dependent targets to set the neon-related compiler flags consistently. As it turns out, this is difficult for non-aarch64 platforms where neon availability is not guaranteed. Hence, for safety (so that we don't cause SIGILL on older generation Android devices), we restrict the neon version of F14 to aarch64. Reviewed By: Maratyszcza, nbronson Differential Revision: D9519198 fbshipit-source-id: cd0c92b17403bce9156a915ad96020f565ca3d6a
-
Dan Melnic authored
Summary: Initialize `std::atomic<std::thread::id> SingletonHolder::creating_thread_`. Reviewed By: yfeldblum Differential Revision: D9525334 fbshipit-source-id: 93831c40bed21d1f3356bd2181e78ba08b7b116f
-
Lewis Baker authored
Add folly::tryEmplaceWith() helper for safely initialising a Try<T> with result of calling a function Summary: Adds a new folly::tryEmplaceWith() function that is the equivalent of makeTryWith() but for in-place initialisation of an existing Try<T> object. Using `tryEmplaceWith(t, func)` rather than `t = makeTryWith(func)` can potentially be more exception safe since the latter could potentially still throw if `Try<T>::operator=()` throws (ie. if T's move-constructor can throw). Updated FiberManager's internals to make use of this to fix a potential exception-safety issue that could have caused a memory leak if the Try<T> move-constructor of the result of fiber threw an exception. Reviewed By: yfeldblum Differential Revision: D9511067 fbshipit-source-id: 827cc88ea1d4fcd505c8d8a9886b87b26db90544
-
Dan Melnic authored
Summary: Fix folly/io/async/test:async_test - AsyncSSLSocketTest.ConnectTFOFallbackTimeout SIGSEGV Reviewed By: djwatson Differential Revision: D9520104 fbshipit-source-id: c1133bcb0617ca7f8d5f8a9ee46080c7c7f5b105
-
Lewis Baker authored
Summary: Add some methods to `folly::Try<T>` to allow the user to in-place construct a value or exception_wrapper inside the Try object at some point after the Try object was constructed. Previously, you had to construct a new temporary `Try<T>` object from the value and move-assign the value. The difference between `t.emplace(args...)` and `tryEmplace(t, args...)` is that the latter catches any exceptions thrown by the constructor and stores the exception in the Try object whereas the former lets the exception propagate out to the caller and leaves the Try object in an empty state. The `Try<void>` implementation now only conditionally constructs the contained exception_wrapper only if hasValue_ is false. This was needed to be able to have the `.emplaceException()` method correctly in-place construct the `exception_wrapper` object. Reviewed By: yfeldblum Differential Revision: D9352439 fbshipit-source-id: f34a4479acb2f93aed11f9626b3152511386bfea
-
Orvid King authored
-
- 27 Aug, 2018 12 commits
-
-
Dan Melnic authored
Summary: Init the std::atomic since in the default constructor is "no initialization takes place" Reviewed By: yfeldblum, djwatson Differential Revision: D9515004 fbshipit-source-id: cdc73c490f77496335e77732b0c7c43c180a54b6
-
Andrii Grynenko authored
Reviewed By: lewissbaker Differential Revision: D9354535 fbshipit-source-id: 49c228e773f21a2fcfa4701eec4cb4dc76c2f990
-
Marshall Cline authored
Summary: This is part of "the great r-valuification of folly::Future": * This is something we should do for safety in general. * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical). * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.). * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect. * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).onTimeout(...)` instead of `f.onTimeout(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics. Reviewed By: LeeHowes Differential Revision: D9441979 fbshipit-source-id: a2f3416bdf481d32d1b94cde1bb713e1afe8ab67
-
Lewis Baker authored
Summary: If the value copy/move constructor threw an exception during copy/move assignment then the Try<T> object would not be left in the empty state. This could lead to double-deletion of resources, or referencing uninitialised memory. Reviewed By: yfeldblum Differential Revision: D9425118 fbshipit-source-id: 6639a0903258ecb031c26264f418fa8794519be3
-
Marshall Cline authored
Summary: This is part of "the great r-valuification of folly::Future": * This is something we should do for safety in general. * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical). * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.). * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect. * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).within(...)` instead of `f.within(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics. Reviewed By: LeeHowes Differential Revision: D9442319 fbshipit-source-id: c751a5e295620bff22c8bdaed57d417bcc0973d1
-
Lee Howes authored
Summary: Makes it invalid to call SemiFuture::defer with a lambda that takes no arguments. Fix to callsites in folly tests that checked this behaviour. Reviewed By: yfeldblum Differential Revision: D9511831 fbshipit-source-id: 11e600cea78887c653e3d0d0e1cd2091c4eb191e
-
Marshall Cline authored
Summary: This is part of "the great r-valuification of folly::Future": * This is something we should do for safety in general. * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical). * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.). * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect. * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).onError(...)` instead of `f.onError(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics. Reviewed By: yfeldblum Differential Revision: D9441402 fbshipit-source-id: 1c39ca7b33bb406d2fd3a995f487693a2d013747
-
Nathan Bronson authored
Summary: F14 previously had a space optimization that relied on the fact that tuple<A,B,C> was std::is_empty if each of A, B, and C was empty (and non-final), but this is not the case on libc++. This diff removes the use of std::tuple and manually packages the hash table functors to enable the Empty Baseclass Optimization. Reviewed By: yfeldblum Differential Revision: D9507243 fbshipit-source-id: 3ee8aa2aa444d91de56210662c86e555406d8eca
-
Dan Zimmerman authored
Summary: We may want to disable exceptions, so lets make this work Reviewed By: yfeldblum Differential Revision: D9381726 fbshipit-source-id: 6e6d022740728a40a5b710ec32309da99f6d0bd0
-
Dan Zimmerman authored
Summary: This doesn't need to be in Memory.h and is one of the reasons Memory.h fails to compile with -fno-exceptions, so split it out I include EnableSharedFromThis.h because I'm not sure how else to ensure backward compatibility any other way (and I have the macro so we can include Memory.h without including EnableSharedFromThis.h) Reviewed By: yfeldblum Differential Revision: D9385709 fbshipit-source-id: bf2c3a757ae4eefba69e6191309d64b347becf12
-
Marshall Cline authored
Summary: This is part of "the great r-valuification of folly::Future": * This is something we should do for safety in general. * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical). * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.). * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect. * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).within(...)` instead of `f.within(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics. Codemod changes: * expr.within(...) ==> std::move(expr).within(...) // if expr is not already an xvalue * expr->within(...) ==> std::move(*expr).within(...) Note: operator precedence of that last step is safe - no need to parenthesize `expr`. Reason: `->` binds more tightly than unary `*`. Reviewed By: yfeldblum Differential Revision: D9511943 fbshipit-source-id: 0cf90e3198453dd997194c2c7f36cc1fa74906f5
-
Yedidya Feldblum authored
Summary: [Folly] `invoke_noreturn_cold`, a helper function for invoking another function with arguments in a `[[noreturn]]` and `[[gnu::cold]]` context. Potentially useful for making the bytecode at the call-site small for icache size or always-inlined fast-path size concerns, when constructing the exception object to be thrown is more complex than passing pre-existing values by reference, such as when computing a message is required. Reviewed By: marshallcline Differential Revision: D9509658 fbshipit-source-id: b8272629876f9e7e81ae0f7a7f06404484d297f2
-
- 26 Aug, 2018 2 commits
-
-
Marshall Cline authored
Summary: This is part of "the great r-valuification of folly::Future": * This is something we should do for safety in general. * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical). * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.). * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect. * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).semi()` instead of `f.semi()`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics. Reviewed By: LeeHowes Differential Revision: D9442704 fbshipit-source-id: 50b1912e33ac23d1a693682fbcd55cd637cdb149
-
Yedidya Feldblum authored
Summary: [Folly] Cut use of boost headers in `sorted_vector_types.h`. Reviewed By: elsteveogrande Differential Revision: D9508079 fbshipit-source-id: 46950a07d1fbc0e316bd6a8cb9766f5d0aeaa74b
-
- 25 Aug, 2018 2 commits
-
-
Yedidya Feldblum authored
Summary: [Folly] Fix `folly/hash/detail/Crc32CombineDetail.cpp` with `-Werror=attributes`. Reviewed By: tehnerd Differential Revision: D9507179 fbshipit-source-id: 67ce235a5e1555973a276de2b4009acab2329ddc
-
Yedidya Feldblum authored
Summary: [Folly] Compute the crc32c and crc32 powers tables at compile time v.s. hardcoding the tables. We translate `gf_multiply_sw` to C++11 `constexpr` form for those cases which still require gcc49. Reviewed By: djwatson Differential Revision: D9495241 fbshipit-source-id: 39aa8b53f7bd8a6dec1ba49013636c57972ef93b
-
- 24 Aug, 2018 6 commits
-
-
Lee Howes authored
Summary: Modify the implementations of thenValue and thenError to be in terms of thenImplementation rather than then. This is a preliminary transformation before splitting then and deprecating parts of it in turn. Reviewed By: marshallcline Differential Revision: D9494305 fbshipit-source-id: 38cd165866f98514ee506dc01f625b7df1db211b
-
Marshall Cline authored
Summary: This is part of "the great r-valuification of folly::Future": * This is something we should do for safety in general. * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical). * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.). * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect. * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).thenMulti(...)` instead of `f.thenMulti(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics. Reviewed By: LeeHowes Differential Revision: D9437627 fbshipit-source-id: f62dc095ce57904057a97c83c958de5227a112a8
-
Andrii Grynenko authored
Differential Revision: D9499697 fbshipit-source-id: b04e9e0d1bf4abf1b5ff497cefbc26da0176eaa2
-
Melissa Winstanley authored
Summary: In some cases, it may be necessary to unicode-escape regular ASCII characters in JSON serialization (example: for JSON sent to browsers that may be interpreted as HTML, "<" should be escaped). Allow additional escape characters to be specified via a bitmap in the serializer options. Reviewed By: yfeldblum, luciang Differential Revision: D8980189 fbshipit-source-id: 000c5279ab0f37a3ee4b2eb38f20afa49dcc5a27
-
Joe Loser authored
Summary: Problem: - There are several tests enabled which rely on using shared_ptr_internals found in folly/concurrency/detail/AtomicSharedPtr-detail.h and the implementation only works for libstdc++ and not with other vendors such as libc++ and libcpp. - This results in a hard error when compiling with Clang with libc++. Solution: - Add a CMake configure check to detect whether one is using libstdc++. FOLLY_USE_LIBSTDCPP macro is set to 1 if using libstdc++, 0 otherwise. - Use FOLLY_USE_LIBSTDCPP to conditionally run the tests relying on shared_ptr_internals only when using libstdc++. Note: - A longer-term solution would be to implement the similar shared_ptr_internals with the libc++ types and provide a fallback locking approach for libcpp (MSVC standard library implementation). Pull Request resolved: https://github.com/facebook/folly/pull/900 Reviewed By: djwatson Differential Revision: D9312236 Pulled By: yfeldblum fbshipit-source-id: 657eddc1e530b00bd5f5b10e7155bc3b6aac3726
-
Yedidya Feldblum authored
Summary: [Folly] Cut unnecessary uses of `Super::` in `F14Policy.h`. Reviewed By: shixiao Differential Revision: D9478746 fbshipit-source-id: ea5b57a66e265ea459a66ff3756a2a54326e73f2
-
- 23 Aug, 2018 9 commits
-
-
Marshall Cline authored
Summary: This is part of "the great r-valuification of folly::Future": * This is something we should do for safety in general. * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical). * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.). * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect. * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).thenMulti(...)` instead of `f.thenMulti(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics. Reviewed By: LeeHowes Differential Revision: D9437767 fbshipit-source-id: f40d0660a50ccfec1040e42c8028efde956a4b48
-
Yedidya Feldblum authored
Summary: [Folly] Work around member alias bugs in some compilers in `F14Policy.h`. MSVC appears to have a hard time with some cases of `using typename Base::MemberAlias;` - replace with `using MemberAlias = typename Base::MemberAlias;`. Reviewed By: shixiao Differential Revision: D9478688 fbshipit-source-id: 5cbf63839cc2b3a085ed633cc4e5703d61990033
-
Sargun Vohra authored
Summary: I find myself wanting something like this pretty often, so I tried my hand at adding it myself. The `groupByAdjacent` operator creates groups bounded wherever the selector changes. It's especially useful for processing sources that've already been sorted on the selector, such as from a database query. Given the following source sequence with keys `A` and `B`: ``` [A1, A2, A3, B1, B2, B3, A4, A5, B4, B5] ``` a regular `groupBy` would return something like: ``` [A:[A1, A2, A3, A4, A5], B:[B1, B2, B3, B4, B5]] ``` while this `groupByAdjacent` would return: ``` [A:[A1, A2, A3], B:[B1, B2, B3], A:[A4, A5], B:[B4, B5]] ``` Given a source where the items are presorted by selector, `groupByAdjacent` should behave identically to `groupBy`, except that `groupByAdjacent` supports infinite sources since it doesn't need to collect the entire source in memory before creating any output. Reviewed By: yfeldblum Differential Revision: D9475326 fbshipit-source-id: 1c8db3abadce5e68394e5fa38bf4bee0b413a03f
-
Orvid King authored
Summary: It is where `std::logic_error` is defined. Fixes https://github.com/facebook/folly/issues/913 Reviewed By: yfeldblum Differential Revision: D9436370 fbshipit-source-id: a4425d7d29f0bd55ea536a49a8923f22944ecbd0
-
Xiao Shi authored
Summary: `F14NodeMap` has proven to be a safe and more performant drop-in replacement for `std::unordered_map`. With the ability to do heterogeneous lookup and mutation, we no longer need `StringKeyedUnorderedMap`, whose main purpose was to avoid unnecessary string copies during lookups and inserts. Reviewed By: nbronson Differential Revision: D9124737 fbshipit-source-id: 81d5be1df9096ac258a3dc1523a6a0f5d6b9e862
-
Ajanthan Asogamoorthy authored
Summary: in the opensource spec I forgot to specify the github org as facebookincubator in D9337956 vs regular facebook, this caused travis builds for proxygen and anything that depends on fizz to fail Reviewed By: knekritz Differential Revision: D9482739 fbshipit-source-id: ba5a8f465c81ec1b9f037114ce1da3614a838699
-
Dan Melnic authored
Summary: ThreadCachedInts use after free fix Reviewed By: djwatson Differential Revision: D9445106 fbshipit-source-id: 989969e54178b5f908f6ca94cde4088ffb2da490
-
Victor Zverovich authored
Summary: `boost::scoped_ptr<T>` is obsolete so replace it with `std::unique_ptr<T[]>`. This also fixes the modular build by eliminating problematic using directives. Reviewed By: shixiao Differential Revision: D9480193 fbshipit-source-id: 8eaa642fe16e91b8f8144f5a10f21729ed984257
-
Marshall Cline authored
Summary: This is part of "the great r-valuification of folly::Future": * This is something we should do for safety in general. * Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical). * This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.). * This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect. * The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).semi()` instead of `f.semi()`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics. Reviewed By: LeeHowes Differential Revision: D9478283 fbshipit-source-id: 7d2f54a8aaa9156b2b274aafc9695fd996f29641
-