1. 07 Aug, 2020 2 commits
    • 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
  2. 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
  3. 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
  4. 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
  5. 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
  6. 02 Aug, 2020 2 commits
  7. 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
  8. 31 Jul, 2020 6 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
    • Robin Cheng's avatar
      Fix a TSAN-detected race condition in TLRefCount. · ca49b1fe
      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
      ca49b1fe
    • Misha Shneerson's avatar
      periodic timers should not cause leaking of folly::RequestContext · 030a288b
      Misha Shneerson authored
      Summary:
      we do have some periodic maintentenance tasks... if those are initialized
      lazily on a requests handling path, they  will capture current rctx and keep it
      it alive forever.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22859344
      
      fbshipit-source-id: 3655d874dee22aaf74b4d80631d4a58e83539752
      030a288b
    • Ajanthan Asogamoorthy's avatar
      -update-app-bytes-written-correctly · 0b2e2a2d
      Ajanthan Asogamoorthy authored
      Summary: AppBytesWitten was no longer updated correctly for AsyncSSLSocket, fix this.
      
      Reviewed By: bschlinker
      
      Differential Revision: D22847139
      
      fbshipit-source-id: 8cabb9883e5ad64c49a283a0d83b8087def4d5ae
      0b2e2a2d
  9. 30 Jul, 2020 4 commits
    • Zhengxu Chen's avatar
      Don't append space delimiter when context string is empty. · 2ee7489b
      Zhengxu Chen authored
      Summary:
      Currently when context string is "", we will still append whitespace to log header. This looks like:
      ```
      I0729 17:36:53.249270 438847 ThriftServerEventHandler.cpp:21 ] Sampling servers
      ```
      which is less readable compared to
      ```
      I0729 17:36:53.249270 438847 ThriftServerEventHandler.cpp:21] Sampling servers
      ```
      . Ideally empty context string should mean "there's no context string to report here", so we should not put an extra delimiter in such case.
      
      Reviewed By: simpkins
      
      Differential Revision: D22835634
      
      fbshipit-source-id: c7fc6de4036050eac3b7d16bf607c637d1394c44
      2ee7489b
    • Shai Szulanski's avatar
      Fix self-assignment in folly::coro::AsyncPipe · f9f5da22
      Shai Szulanski authored
      Reviewed By: yfeldblum
      
      Differential Revision: D22843001
      
      fbshipit-source-id: 55f48de90f16c53983ea94c8e1102c5b33e25195
      f9f5da22
    • Krzysztof Kozielczyk's avatar
      Add missing return in AsyncPipe move assignment operator · 10d69890
      Krzysztof Kozielczyk authored
      Summary:
      I'm using AsyncPipe heavily in D22593723. At one iteration I needed to assign with move a holding object and it wouldn't compile because the operator doesn't return any value
      
      Here's the error: `folly/experimental/coro/AsyncPipe.h:54:3: error: control reaches end of non-void function [-Werror,-Wreturn-type]`
      
      Reviewed By: yfeldblum
      
      Differential Revision: D22593724
      
      fbshipit-source-id: 53f27b95c1dff0068a375291d4ee6f445f892dd9
      10d69890
    • Yedidya Feldblum's avatar
      Move an exception_ptr in exceptionStr · e050deab
      Yedidya Feldblum authored
      Summary: [Folly] Move an `exception_ptr` in `exceptionStr`, which should be ever-so-slightly cheaper than copying it since that may avoid atomic refcount operations. It's not much cheaper overall because the throw/catch machinery remains expensive.
      
      Reviewed By: ot, luciang
      
      Differential Revision: D22529858
      
      fbshipit-source-id: 7e5d4ec5c1ca601a766c63133ae759651ae7be85
      e050deab
  10. 29 Jul, 2020 2 commits
    • Yedidya Feldblum's avatar
      Outline exceptionStr function bodies · 0df06cbe
      Yedidya Feldblum authored
      Summary: [Folly] Outline `exceptionStr` function bodies and move them to the `.cpp`.
      
      Reviewed By: ot, luciang
      
      Differential Revision: D22529702
      
      fbshipit-source-id: 165c14009f224e86e63b3a3126c8b837cf3793f5
      0df06cbe
    • Yedidya Feldblum's avatar
      Skip the safe-assert dep on FileUtil · ab1c217f
      Yedidya Feldblum authored
      Summary: [Folly] Skip the safe-assert dep on FileUtil since it really needs only the combinators.
      
      Reviewed By: Orvid
      
      Differential Revision: D22699899
      
      fbshipit-source-id: 73485439e397c10210d09eacbb38116e3e0a50a0
      ab1c217f