- 08 Aug, 2020 2 commits
-
-
Xun Li authored
Summary: Add definition to EventBase. Differential Revision: D23004586 fbshipit-source-id: 828767b3d9a43c33bcf047b9b06c08bfba8fde68
-
Misha Shneerson authored
Reviewed By: andriigrynenko Differential Revision: D23008623 fbshipit-source-id: db3e5a3dfe8dbbf1c9583b8e4924da31c2f5c174
-
- 07 Aug, 2020 8 commits
-
-
Chad Austin authored
Summary: The implementation of `std::basic_string` with the legacy ABI and with `_GLIBCXX_FULLY_DYNAMIC_STRING`, as appears by default on CentOS 7, incorrectly requires the provided allocator type to have a default constructor. The `Allocator` concept does not require this and many allocator types with runtime state shared between copies will not provide this. Switch to `folly::fbstring` in `ElfCache` to sidestep this problem. Fixes: https://github.com/facebook/folly/issues/1344. Reviewed By: yfeldblum Differential Revision: D22970418 fbshipit-source-id: 1e188d668991d25f53da5702b31f4a7a7ec7d7e8
-
Misha Shneerson authored
Summary: (Note: this ignores all push blocking failures!) Reviewed By: andriigrynenko Differential Revision: D23005233 fbshipit-source-id: 4ea528505e06fec0af7841ccbd6d41eabe0de558
-
Robin Cheng authored
Summary: A tracepoint semaphore address is written to the notes section as either the absolute address (if binary is compiled as non-PIE) or the relative address (if binary is compiled as PIE) of the semaphore. In the former case, the test passes; in the latter case, the test as it was could not possibly pass because the binary is mapped to some random base address. This diff makes a guess: if the addresses are equal, then we're non-PIE; otherwise check if the address offset is valid - that offset is the address at which the binary is mapped, so it always begins with the ELF header, so we can check against that. If someone knows how to easily figure out that random address or to detect whether the binary is PIE (thereby disabling the test) that'd be great too. This fixes TSAN for the test, as TSAN binaries are built as PIE. Reviewed By: yfeldblum Differential Revision: D22752824 fbshipit-source-id: cd320f9fb8355a20f65e2385f0a1ab1c24ec0b81
-
Sotirios Delimanolis authored
Summary: Previously `OpenSSLUtils::getCommonName` was allocating a 64 byte long string and copying the common name from the `X509` into it. We unexpectedly returned that string potentially containing trailing `\0`s. This diff rewrites the function to allocate a big enough buffer, write to it, then return an appropriately trimmed new string from it. Reviewed By: yfeldblum Differential Revision: D22910431 fbshipit-source-id: 77b4fc3ccde4f7ddb0f62b884ecd7ff0c868c1ca
-
TJ Yin authored
Reviewed By: yfeldblum, andriigrynenko Differential Revision: D22965140 fbshipit-source-id: 8f9996ccc28aff621879774e901b0c75e8c62883
-
Maged Michael authored
Summary: Make hazptr_obj_cohort active_ atomic to be data-race-free. Reviewed By: yfeldblum Differential Revision: D22976574 fbshipit-source-id: 5a09aa7cedc26c71b2160b2df14a7e0f7b55e2e7
-
Paul Jakma authored
Summary: The deps for Fedora in the public README are a little out of date. Add / update some. Reviewed By: yfeldblum Differential Revision: D22951514 fbshipit-source-id: 645af3df1b71dcdab06408d73d99c34c482ea534
-
Chris Keeline authored
Summary: When we do the context switch onto the fiber, the stack frame points back to the main context stack. While the fiber is running, this allows backtraces to continue back onto the main context. When the fiber is suspended, we execute code on the main context's stack, the pointers to this stack are now garbage, backtracing in gdb runs into problems, such as garbage frames, gdb crashes, and occasional cycles. The fix here is to write a 0 to the saved return address in the last frame, when a fiber suspends. gdb detects this and stops the trace #38 folly::fibers::FiberManager::activateFiber(folly::fibers::Fiber*) (this=0x0, fiber=0x0) at folly/fibers/FiberManagerInternal-inl.h:82 #39 folly::fibers::FiberManager::runReadyFiber(folly::fibers::Fiber*) (this=0x0, fiber=0x0) at folly/fibers/FiberManagerInternal-inl.h:127 #40 0x0000000000000000 in () I gated the fix to linux since I'm not sure about the ABI for other platforms. Reviewed By: yfeldblum Differential Revision: D22774189 fbshipit-source-id: 2d4ad67a4a01bebc4ff1f9c285f6e3e95d13432c
-
- 06 Aug, 2020 10 commits
-
-
Cooper Lees authored
Summary: - Move to the newer and forward moving libzstd-dev https://packages.ubuntu.com/search?suite=default§ion=all&arch=any&keywords=libzstd&searchon=names Reviewed By: wez Differential Revision: D22981784 fbshipit-source-id: 3647357aad60f2335ea494c35c174e9ffec61346
-
Robin Cheng authored
Summary: The find path reads a node that the insert path writes; they need to have acquire/release semantics due to the node being allocated on the fly. Reviewed By: yfeldblum, magedm Differential Revision: D22937394 fbshipit-source-id: c1497ed1764cd43e396bc520e9b716cdfe29e5ca
-
Xavier Deguillard authored
Summary: Redirecting stdout means that ninja/cmake won't act as if it's invoked interactively, ie: it will buffer the output, show every single files being compiled (instead of just a line or progress), etc. This results in a fairly janky UX when getdeps is used on the command line. By not redirecting stdout, we get immediate feedback about the tests being run, and the files being compiled. Reviewed By: wez Differential Revision: D22967815 fbshipit-source-id: 872ddbf421065686c384a3a876d0acb8832baf2e
-
Yedidya Feldblum authored
Summary: [Folly] Prefer `std::condition_t` vs preprocessor in `AsyncSocket`. Reviewed By: bschlinker, lnicco Differential Revision: D22975614 fbshipit-source-id: 562d1b794ec3dbab538ad507b2ec5d0fe07c7757
-
Yedidya Feldblum authored
Summary: [Folly] Let `exceptionStr` peek into unknown exception types when building with libstdc++ via `std::exception_ptr::__cxa_exception_type()`. Reviewed By: ot, luciang Differential Revision: D22521812 fbshipit-source-id: b75811ed4159df81e651c5df1386c406f6ac6d60
-
Shai Szulanski authored
Summary: Generic code taking in a `Task<void|T>` cannot easily forward the result without throwing exceptions because the `return_value` overload taking a `Try` cannot be added to `Task<void>` which already has `return_void`. Instead we use `yield_value` as we did with `co_error` to make the desired generic code possible. Reviewed By: lewissbaker Differential Revision: D22934887 fbshipit-source-id: ec50de5832f9038d92749bdde2c1d43244288a3c
-
Dylan Yudaken authored
Summary: We know ahead of time the size of this list, so may as well pre-allocate space for it Reviewed By: yfeldblum Differential Revision: D22838412 fbshipit-source-id: 7d70495fe0910e65dfea49fe7baf675308ab9331
-
Robin Cheng authored
Summary: If the binary is compiled as PIE, such as in the case of TSAN, the kStringValue symbol's value, which is an address that in turn points to the "coconuts" character array, may be stored as 0 in the binary file, and rely on the dynamic linker to relocate at loading time. The corresponding relocation entry (in the .rela.dyn section) contains the relative offset instead. We do not seem to have implemented parsing of the relocation section, so this diff changes the test to be more lenient, and upon seeing a zero will just verify the address of the symbol against the live binary. Reviewed By: yfeldblum Differential Revision: D22782080 fbshipit-source-id: ffbfc7c23b84865fb29bbfb782feb896d687594b
-
Dan Melnic authored
Summary: Add the ability to set the event EventRecvmsgCallback Reviewed By: danobi Differential Revision: D22959687 fbshipit-source-id: 274cecf4ab4b8015d5e7e725c7e899f603cea7a0
-
Yedidya Feldblum authored
Summary: [Folly] Add missing include in `folly/ExceptionString.cpp`. Reviewed By: ot, luciang Differential Revision: D22529764 fbshipit-source-id: 91af74023fd81b1882a19889fd92fda4e1bdfcc7
-
- 05 Aug, 2020 5 commits
-
-
Robin Cheng authored
Summary: Two issues: - Executor not properly shutdown before destruction - Shadow stack overflow due to TSAN not handling fibers correctly. For this we just reduce the number of tasks for now; the proper fix would be likely quite involved and it's not clear whether it's worth it at this point since I think it only affects diagnostic messages that TSAN prints. Reviewed By: yfeldblum Differential Revision: D22901657 fbshipit-source-id: 833454e6f54ab6fc7c53333157197345e8d887e2
-
Ravindra Sunkad authored
Summary: Environment variable WITH_ASAN can be set to enable building with ASAN options. Ran into issues with getdeps.py extensions P137131160. Once that is resolved will do away with the ENV approach Requires building folly lib also with ASAN to avoid linking issues Reviewed By: shri-khare Differential Revision: D22860508 fbshipit-source-id: 48dea49378319972894f69b11bac6fa63ee20db9
-
Dylan Yudaken authored
Summary: This spin lock is not required, and guards quite a complex region that could easily fallback to the slow path Reviewed By: andriigrynenko Differential Revision: D22839268 fbshipit-source-id: 045a3562a2d4358b4c95e155a4b9958b2b7f67d4
-
Robin Cheng authored
Summary: Two ordering issues: setTreeNode cannot store the node as relaxed, because the node is newly allocated before storing, and if another thread gets the node pointer value, relaxed ordering cannot guarantee that the node has been fully initialized before it. There are two ways this node can be read, one via optimisticReadValue, which loads with memory_order_acquired, which would now correctly synchronize with the release; the other is readValue, which I think was intended to be used only when the lock corresponding to the node is held, but there is one case on line 781 in forceInsert where we fail to acquire the lock but it looks like we want to retry unless we're out of capacity (on a best effort basis) and therefore we call readValue to check the capacity (on a best effort basis), so it looks as if memory_order_relaxed is okay, but it is not, because the node is a pointer that may not be fully initialized with relaxed ordering. So I'm changing that to memory_order_acquire as well. In general it's not really valid to use relaxed ordering if the value is a pointer and the pointee is allocated on the fly. Reviewed By: yfeldblum Differential Revision: D22936002 fbshipit-source-id: 1c8a000b6033e4a81df5f9a5e850cfe350dbeb15
-
Dan Melnic authored
Summary: Remove TSAN code that caused an assert Reviewed By: andriigrynenko Differential Revision: D22925613 fbshipit-source-id: 67cf634ccd29453a6e399951b8bf541a2c471104
-
- 04 Aug, 2020 4 commits
-
-
Chad Austin authored
Summary: The SymbolizePrinter classes are portable to any unix, so split them into their own target. Reviewed By: yfeldblum, luciang Differential Revision: D22883912 fbshipit-source-id: 3bce6d6584182b818bf803da187eae56477bac8a
-
Chad Austin authored
Summary: In the interest of using SignalHandler on macOS, which does not use ELF, break some dependencies and split the targets. Reviewed By: yfeldblum, luciang Differential Revision: D22883896 fbshipit-source-id: e576b6e1684522fc1665c864780faea882d0263a
-
Robin Cheng authored
Summary: The memory ordering when loading the `waiter_` atomic must be at least memory_order_acquire to ensure that anything sequenced after timedWaitThread happens before the corresponding post call. In fact, this is already the memory ordering used in the non-timed version Baton::waitThread. Differential Revision: D22901656 fbshipit-source-id: 8e8156bdfda02d6b26565b91054ef3cc59a28e8a
-
Dan Melnic authored
Summary: Add support for TimerFD async recv Reviewed By: kevin-vigor Differential Revision: D22860507 fbshipit-source-id: e1afc9d72608f5a574413e1230d6665420240af7
-
- 03 Aug, 2020 3 commits
-
-
Michael Shao authored
Summary: Add APIs to disable rx/tx checksum for UDP over IPV6 Differential Revision: D22487077 fbshipit-source-id: 22af21b108e84782ec0a76ccbc49eaafa73bd266
-
Robin Cheng authored
Summary: Mcrouter has a logger thread which queries the fiber manager for 3 stats; these stats are non-atomic and therefore this causes a race. This diff applies a simple fix to make these atomic. It does not feel like this is the best fix though, so let me know if anyone has ideas for a better fix. It feels that stats should be provided some other way (rather than a bunch of atomics)... Reviewed By: yfeldblum Differential Revision: D22878457 fbshipit-source-id: f17544b38836c454162b83cfa570c90c5e7e6b9a
-
Parvez Shaikh authored
Summary: port serdes object type is introduced in 1.5.2 upgrading tp2 sai dependency to 1.5.2 note that for first time, oss build failed, i had to back out removing http_proxy and https_proxy indicated here - D17429928 Differential Revision: D22888202 fbshipit-source-id: c0f90b1caa01603d25b0559188ae961df1dd13d5
-
- 02 Aug, 2020 2 commits
-
-
Robin Cheng authored
Summary: TSAN has a limit of 8128 threads. Reviewed By: yfeldblum Differential Revision: D22888242 fbshipit-source-id: 12b78e11145de433dfa31ea1480649af7f8dac57
-
Andy Wei authored
Summary: Simplify the interface as the touch(), remove() methods are not needed now. Reviewed By: therealgymmy Differential Revision: D22547658 fbshipit-source-id: 6d5c799fb21cb4cb6ee20203c820f4d6ff7e8255
-
- 01 Aug, 2020 2 commits
-
-
Robin Cheng authored
Summary: Like ASAN, TSAN also maps large memory regions. So disable the rlimit for TSAN as well. Differential Revision: D22743326 fbshipit-source-id: 5023b66f1526894c77e6c693a2481f6ade388b9e
-
Yedidya Feldblum authored
Summary: [Folly] Avoid `std::forward` in `folly/futures/` since function template instantiation is expensive at compile time and `std::forward` is, when used correctly, nothing more than library sugar for straightforward syntax. Reviewed By: LeeHowes Differential Revision: D22673234 fbshipit-source-id: f88bc8c8aa13797ae6eccf1a421a47ff135589e1
-
- 31 Jul, 2020 4 commits
-
-
Nathan Bronson authored
Summary: Some of the CHECKs and DCHECKs in F14 are often tripped by users that write to an F14 map or set from multiple threads without proper concurrency control. This diff adds a message to those locations that suggests that they may have a race condition. Reviewed By: yfeldblum Differential Revision: D22826369 fbshipit-source-id: 8d5e96feea4dbb8dda404980d9a9d3412fb2eb64
-
Robin Cheng authored
Summary: See task T70610458 description for the TSAN error reported. The scenario that can trigger this race condition is: * Thread A has a ThreadEntryNode N whose `next` points to thread B's ThreadEntry. * Thread B exits, which causes it to delete its ThreadEntry and all its nodes; therefore writing `N->next = <the next entry after B>`. * At the same time, thread A accesses the thread local corresponding to N; when checking whether the ThreadEntryNode is already added to the linked list, it checks whether the `next` pointer is null; this read races with the above write. Even though this is a technically a race condition, it is unlikely to actually trigger any negative effect in practice, because at thread exit, the overwrite of `N->next = ...` will never assign a nullptr, so it will never affect the outcome of `if (!next)` in thread A. Only the thread that owns the ThreadEntryNode has the ability to change between `next == nullptr` and `next != nullptr`. (The former is called "zero" in the code). Nonetheless, C++ says "race condition leads to undefined behavior", so it's still good to fix... It's not clear what the best way to fix this is. Here I've presented one fix, which is to introduce another field in ThreadEntryNode that is private to the thread and simply indicates whether the node is zero; this field is strictly accessed by only the owning thread and therefore will not cause a race condition. I used a bit in `id` to not increase memory consumption, but maybe that's not important - I don't know. I'm also unsure about the performance implications. Let me know if anyone has different ideas; I'm very open to suggestions here. Reviewed By: yfeldblum Differential Revision: D22743031 fbshipit-source-id: 12722ca241a224b546a0f6763b125e922813c844
-
Robin Cheng authored
Summary: Turned it into a thread_local instead. Only affects tests. Reviewed By: yfeldblum Differential Revision: D22744469 fbshipit-source-id: cbe8263d560f5abad27c7e61b1963aecf090a94e
-
Robin Cheng authored
Summary: See RefCountTest.cpp for a newly added test. When one thread is decrementing a local refcount and another thread is decrementing a global refcount, and the global decrement results in global zero, the second thread can reasonably expect to be able to safely destroy the refcount object. However, a memory ordering issue in LocalRefCount::collect() causes TSAN to detect a write-delete race condition on the inUpdate_ variable (though that variable is not the cause of the issue). This diff changes collect() to use memory_order_acquire so that upon reading false, collect() may exit while assuming that all memory accesses in update() are complete. This way, any memory accesses (such as a read of refCount_.state_ in update()) happen before a delete of the whole TLRefCount like so: - A memory access in update() is sequenced before the release store of false into inUpdate_ at the SCOPE_EXIT of the function, - which synchronizes with the acquire load at the end of collect() - which is sequenced before the deletion that can only happen after collect() Without this diff, the second relationship above is broken and TSAN complains. I also updated the comment on top of inUpdate_ to better reflect the purpose. I couldn't understand the original comment (which seems to suggest it's related to refcount *correctness* rather than a write-delete race), but maybe I missed something there. Reviewed By: yfeldblum Differential Revision: D22805717 fbshipit-source-id: 34cf72676760526ba457f939e307ed03dc722528
-