1. 09 Aug, 2020 1 commit
  2. 08 Aug, 2020 2 commits
  3. 07 Aug, 2020 8 commits
    • Chad Austin's avatar
      Avoid std::string with reentrant_allocator in ElfCache · abb4ece6
      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
      abb4ece6
    • Misha Shneerson's avatar
      fix buffer overrun · b19f4306
      Misha Shneerson authored
      Summary: (Note: this ignores all push blocking failures!)
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D23005233
      
      fbshipit-source-id: 4ea528505e06fec0af7841ccbd6d41eabe0de558
      b19f4306
    • Robin Cheng's avatar
      Make //folly/tracing/test:static_tracepoint_test work if compiled as PIE. · d6085636
      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
      d6085636
    • Sotirios Delimanolis's avatar
      Fix OpenSSLUtils::getCommonName to correctly trim trailing null characters · 4892f1bb
      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
      4892f1bb
    • TJ Yin's avatar
      make time based unit-test less flaky by waiting longer · 85a8672b
      TJ Yin authored
      Reviewed By: yfeldblum, andriigrynenko
      
      Differential Revision: D22965140
      
      fbshipit-source-id: 8f9996ccc28aff621879774e901b0c75e8c62883
      85a8672b
    • Maged Michael's avatar
      hazptr: Make cohort active_ data-race-free · f2d225e4
      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
      f2d225e4
    • Paul Jakma's avatar
      folly/README: Update the Fedora deps in the public README · 0bf08a92
      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
      0bf08a92
    • Chris Keeline's avatar
      Fix fiber backtraces in gdb · 30dcb7d8
      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
      30dcb7d8
  4. 06 Aug, 2020 10 commits
    • Cooper Lees's avatar
      Update zstd dev debian package · d0302913
      Cooper Lees authored
      Summary:
      - Move to the newer and forward moving libzstd-dev
      https://packages.ubuntu.com/search?suite=default&section=all&arch=any&keywords=libzstd&searchon=names
      
      Reviewed By: wez
      
      Differential Revision: D22981784
      
      fbshipit-source-id: 3647357aad60f2335ea494c35c174e9ffec61346
      d0302913
    • Robin Cheng's avatar
      Fix an atomic memory ordering issue in SIMD version of the ConcurrentHashMap. · 015949ec
      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
      015949ec
    • Xavier Deguillard's avatar
      runcmd: do not pipe stdout on a tty · bdf5690a
      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
      bdf5690a
    • Yedidya Feldblum's avatar
      Prefer std::condition_t vs preprocessor in AsyncSocket · cbe8ef9d
      Yedidya Feldblum authored
      Summary: [Folly] Prefer `std::condition_t` vs preprocessor in `AsyncSocket`.
      
      Reviewed By: bschlinker, lnicco
      
      Differential Revision: D22975614
      
      fbshipit-source-id: 562d1b794ec3dbab538ad507b2ec5d0fe07c7757
      cbe8ef9d
    • Yedidya Feldblum's avatar
      Let exceptionStr peek into unknown exception types · ec55993d
      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
      ec55993d
    • Shai Szulanski's avatar
      Add yield_value(Try<>) to coro::Task · dde95013
      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
      dde95013
    • Dylan Yudaken's avatar
      pre-allocate freeList · 5076d050
      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
      5076d050
    • Robin Cheng's avatar
      Make folly/experimental/symbolizer/test:elf_tests work if compiled as PIE. · e0bd7f09
      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
      e0bd7f09
    • Dan Melnic's avatar
      Add the ability to set the event EventRecvmsgCallback · 45cd2ae5
      Dan Melnic authored
      Summary: Add the ability to set the event EventRecvmsgCallback
      
      Reviewed By: danobi
      
      Differential Revision: D22959687
      
      fbshipit-source-id: 274cecf4ab4b8015d5e7e725c7e899f603cea7a0
      45cd2ae5
    • Yedidya Feldblum's avatar
      Add missing include in folly/ExceptionString.cpp · e7e80abf
      Yedidya Feldblum authored
      Summary: [Folly] Add missing include in `folly/ExceptionString.cpp`.
      
      Reviewed By: ot, luciang
      
      Differential Revision: D22529764
      
      fbshipit-source-id: 91af74023fd81b1882a19889fd92fda4e1bdfcc7
      e7e80abf
  5. 05 Aug, 2020 5 commits
    • Robin Cheng's avatar
      Fix TSAN issues with FibersTest. · 11018acb
      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
      11018acb
    • Ravindra Sunkad's avatar
      Add option to build with ASAN on OSS build · 0cb1f60c
      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
      0cb1f60c
    • Dylan Yudaken's avatar
      remove unneeded spinlock · 09e27c74
      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
      09e27c74
    • Robin Cheng's avatar
      Fix atomic memory orderings in RelaxedConcurrencyPriorityQueue. · 737ccb70
      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
      737ccb70
    • Dan Melnic's avatar
      Remove TSAN code that caused an assert · f711b660
      Dan Melnic authored
      Summary: Remove TSAN code that caused an assert
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D22925613
      
      fbshipit-source-id: 67cf634ccd29453a6e399951b8bf541a2c471104
      f711b660
  6. 04 Aug, 2020 4 commits
    • Chad Austin's avatar
      split SymbolizePrinter into its own target · 52264b66
      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
      52264b66
    • Chad Austin's avatar
      split Symbolizer and SignalHandler into their own targets · 78f06465
      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
      78f06465
    • Robin Cheng's avatar
      Fix a memory ordering issue in Baton::timedWaitThread. · 1c320bc9
      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
      1c320bc9
    • Dan Melnic's avatar
      Add support for TimerFD async recv · 55917d46
      Dan Melnic authored
      Summary: Add support for TimerFD async recv
      
      Reviewed By: kevin-vigor
      
      Differential Revision: D22860507
      
      fbshipit-source-id: e1afc9d72608f5a574413e1230d6665420240af7
      55917d46
  7. 03 Aug, 2020 3 commits
    • Michael Shao's avatar
      Add API to disable UDP6 checksum · 8ca7ae74
      Michael Shao authored
      Summary: Add APIs to disable rx/tx checksum for UDP over IPV6
      
      Differential Revision: D22487077
      
      fbshipit-source-id: 22af21b108e84782ec0a76ccbc49eaafa73bd266
      8ca7ae74
    • Robin Cheng's avatar
      Attempt to fix a race condition around fiber stats · 5d134e78
      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
      5d134e78
    • Parvez Shaikh's avatar
      SAI API 1.5.2 · f893bb0a
      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
      f893bb0a
  8. 02 Aug, 2020 2 commits
  9. 01 Aug, 2020 2 commits
    • Robin Cheng's avatar
      Fix TSAN crash for RetryingTest. · ab8339ea
      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
      ab8339ea
    • Yedidya Feldblum's avatar
      Avoid std::forward in folly/futures/ · c5cff936
      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
      c5cff936
  10. 31 Jul, 2020 3 commits
    • Nathan Bronson's avatar
      suggest race condition to users that trip F14's internal checks · 28da027e
      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
      28da027e
    • Robin Cheng's avatar
      Attempt to fix a race condition in ThreadLocal related to ThreadEntryNode. · 2fb430a3
      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
      2fb430a3
    • Robin Cheng's avatar
      Fix race condition in //folly/test:synchronized_test due to sharing RNG on multiple threads. · eb493d16
      Robin Cheng authored
      Summary: Turned it into a thread_local instead. Only affects tests.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22744469
      
      fbshipit-source-id: cbe8263d560f5abad27c7e61b1963aecf090a94e
      eb493d16