1. 23 Feb, 2016 1 commit
    • Lucian Grijincu's avatar
      folly: symbolizer: dwarf: don't fallback to linear scan if address missing from .debug_aranges · efe4f93c
      Lucian Grijincu authored
      Summary:Presence of .debug_aranges implies user expects fast address lookup.
      Some addresses might not be avaialble in .debug_aranges.
      
      Don't do slow lookup in .debug_info, as it can lead to unexpected slowdowns.
      
      override-unit-failures
      
      Reviewed By: philippv
      
      Differential Revision: D2965323
      
      fb-gh-sync-id: 405daefd57cdff4344fd231c5f5b7ff4dcd9df8c
      shipit-source-id: 405daefd57cdff4344fd231c5f5b7ff4dcd9df8c
      efe4f93c
  2. 22 Feb, 2016 3 commits
    • Neel Goyal's avatar
      Fix OSX build · d45626d0
      Neel Goyal authored
      Summary: Fixed the build using the homebrew instructions
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2960409
      
      fb-gh-sync-id: 218b879f26b3baab09676cac51787bf36d0b4bed
      shipit-source-id: 218b879f26b3baab09676cac51787bf36d0b4bed
      d45626d0
    • Nathan Bronson's avatar
      fix SIOF in CacheLocality.h's AccessSpreader · bf385e23
      Nathan Bronson authored
      Summary:This diff moves all data accessed during
      AccessSpreader<>::current(x) into the .data segment, avoiding SIOF
      without adding indirection or dynamic gating as would be the case for
      normal singleton-like constructs.  The diff also trims the AccessSpreader
      API to include only those methods that people actually seem to use.
      
      Reviewed By: djwatson
      
      Differential Revision: D2945205
      
      fb-gh-sync-id: 847e31adc4450217f4ed0575686be261fb504d7c
      shipit-source-id: 847e31adc4450217f4ed0575686be261fb504d7c
      bf385e23
    • Michael Lee's avatar
      Remove extra `int main`s from unit tests. · d31eb712
      Michael Lee authored
      Summary: test/common/TestMain.cpp should provide the necessary initialization.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2947245
      
      fb-gh-sync-id: 75b9d241673667badf4dc8fb81285c60cbb8166a
      shipit-source-id: 75b9d241673667badf4dc8fb81285c60cbb8166a
      d31eb712
  3. 20 Feb, 2016 2 commits
    • Andrii Grynenko's avatar
      Move RequestContext::getStaticContext to .cpp · bb5ed807
      Andrii Grynenko authored
      Summary: All singleton registration has to happen in .cpp. Otherwise singleton may be double registered if inlined.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2956951
      
      fb-gh-sync-id: bea425c31270d614f4b8a780204694168602fe86
      shipit-source-id: bea425c31270d614f4b8a780204694168602fe86
      bb5ed807
    • Andrii Grynenko's avatar
      Support nested FiberManagers · 743a0b48
      Andrii Grynenko authored
      Summary:It's possible to have nested loopUntilNoReady (with different FiberManagers). See unit test for a simple example.
      This diff fixes currentFiberManager_ to be a stack.
      
      Reviewed By: meyering
      
      Differential Revision: D2953468
      
      fb-gh-sync-id: 0abdcb7f43c94e7bb0adef8440699dc8e138d21a
      shipit-source-id: 0abdcb7f43c94e7bb0adef8440699dc8e138d21a
      743a0b48
  4. 19 Feb, 2016 6 commits
    • Andrii Grynenko's avatar
      Fix EventBaseOnDestructionCallback to loop FiberManager · 95ade4c9
      Andrii Grynenko authored
      Summary: EventBase loop should do it as well, but it's possible for LoopCallback to be removed before EventBaseOnDestructionCallback.
      
      Reviewed By: pavlo-fb
      
      Differential Revision: D2951303
      
      fb-gh-sync-id: 43f5a91e7ecffa7ab7feec32fa45e01b28db66a3
      shipit-source-id: 43f5a91e7ecffa7ab7feec32fa45e01b28db66a3
      95ade4c9
    • Andrii Grynenko's avatar
      Fix folly::Singleton to work in dynamically linked binaries · f390a90d
      Andrii Grynenko authored
      Summary:This implements StaticSingletonManager which is then used to create all leaked Meyers singletons.
      
      StaticSingletonManager is a singleton itself, which is created in a separate compilation unit (Singleton.cpp) and so we can be sure that other compilation units will always see a single instance of StaticSingletonManager, even if linked dynamically.
      
      StaticSingletonManager then keeps a dictionary of typeid -> object pointer, which is used to de-duplicate same singleton being re-created from different compilation units (linked dynamically), usually because of code inlining.
      
      override-unit-failures
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2913027
      
      fb-gh-sync-id: 1f5015a79a7a8297ebf5f0fe3fd0cc7eb44f706b
      shipit-source-id: 1f5015a79a7a8297ebf5f0fe3fd0cc7eb44f706b
      f390a90d
    • Tianjiao Yin's avatar
      fix dangling pointer bug in IPAddressV6 · ac31e5f5
      Tianjiao Yin authored
      Summary: `mask(numBits)` returns a temporary variable. After its lifetime ends, subbytes becomes a dangling pointer.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2953699
      
      fb-gh-sync-id: 5d5a35716ecaa1b3d96edb5a459615756848b92f
      shipit-source-id: 5d5a35716ecaa1b3d96edb5a459615756848b92f
      ac31e5f5
    • Yedidya Feldblum's avatar
      Indestructible · 7bf18354
      Yedidya Feldblum authored
      Summary:[Folly] `Indestructible`.
      
      For when you need a Meyers singleton that will never be destructed, even at program exit.
      
      Good for large data structures with many heap allocations, as long as they have no behavior. No point in ever destructing them.
      
      HT: Proxygen, and Orvid.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D2947959
      
      fb-gh-sync-id: 7dd1f725edf137ba81fbf4032a0819fd7c627261
      shipit-source-id: 7dd1f725edf137ba81fbf4032a0819fd7c627261
      7bf18354
    • Lucian Grijincu's avatar
      folly: symbolizer: slow address->{file+line-number} lookup if `.debug_aranges`... · 4d4ca9ec
      Lucian Grijincu authored
      folly: symbolizer: slow address->{file+line-number} lookup if `.debug_aranges` is missing (e.g. --strip-debug-non-line)
      
      Summary:Binaries linked with `gold` and `--strip-debug-non-line` don't have an `.debug_aranges` section
      
      We still want to map `address->{file+line-number}` to get nice stack
      traces even though this might be slower (linear search all compilation unit entries in `.debug_info`).
      
      Before:
      ```
      $ # link with gold + --strip-debug-non-line
      $ folly/experimental/exception_tracer/exception_tracer_test
      E0217 15:02:13.694947 1321814 ExceptionTracer.cpp:179] Exception type: std::runtime_error (9 frames)
          @ 000000000040ad2d __cxa_throw
          @ 0000000000409df3 bar()
          @ 0000000000409eab baz()
          @ 0000000000407c77 main
          @ 00007f00dd9860f5 __libc_start_main
          @ 000000000040991b (unknown)
      ```
      
      After (similar to the output without `--strip-debug-non-line`):
      ```
      E0217 18:37:37.579596 1583124 ExceptionTracer.cpp:179] Exception type: std::runtime_error (9 frames)
          @ 000000000040ad6d __cxa_throw
                             ./folly/experimental/exception_tracer/ExceptionTracerLib.cpp:57
          @ 0000000000409e33 bar()
                             ./folly/experimental/exception_tracer/ExceptionTracerTest.cpp:24
          @ 0000000000409eeb baz()
                             ./folly/experimental/exception_tracer/ExceptionTracerTest.cpp:51
          @ 0000000000407c87 main
                             ./folly/experimental/exception_tracer/ExceptionTracerTest.cpp:96
          @ 00007f1d16ff80f5 __libc_start_main
          @ 000000000040995b (unknown)
      ```
      
      Differential Revision: D2947965
      
      fb-gh-sync-id: e517bab324b1dcb70cadc9a5211ce794e35c83a5
      shipit-source-id: e517bab324b1dcb70cadc9a5211ce794e35c83a5
      4d4ca9ec
    • Christopher Dykes's avatar
      Don't use a VLA for the double->string buffer. · ad925ffa
      Christopher Dykes authored
      Summary: MSVC is actually smarter about this than GCC, as MSVC doesn't support VLAs, it tries to eval the length at compile time. GCC on the other hand doesn't try to eval it at compile time, resulting in compiles via CMake telling us that this is a VLA.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2911929
      
      fb-gh-sync-id: ffaa133bcf4129a3e02f7e875966d3ae6a97be6a
      shipit-source-id: ffaa133bcf4129a3e02f7e875966d3ae6a97be6a
      ad925ffa
  5. 18 Feb, 2016 2 commits
    • Sven Over's avatar
      folly/ApplyTuple.h: fix const-correctness & other issues, simplify · ca09289d
      Sven Over authored
      Summary:The existing implementation of folly::applyTuple does not support
      mutable callables (such as mutable lambdas and other functor objects
      that only implement non-const operator()).
      
      This commit adds a few more unit tests and changes the implementation
      so that new and existing tests pass.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2942622
      
      fb-gh-sync-id: 82478f290e9fd2020358ff79ef0a6bcf8a43738c
      shipit-source-id: 82478f290e9fd2020358ff79ef0a6bcf8a43738c
      ca09289d
    • Yedidya Feldblum's avatar
      Sort the Makefile headers a bit · 72cab06f
      Yedidya Feldblum authored
      Summary:[Folly] Sort the `Makefile` headers a bit.
      
      Just manually moving a few obviously out-of-order entries.
      
      Reviewed By: Orvid
      
      Differential Revision: D2945668
      
      fb-gh-sync-id: 604976d25e5913ef01d591e23737d61dacfbfc86
      shipit-source-id: 604976d25e5913ef01d591e23737d61dacfbfc86
      72cab06f
  6. 17 Feb, 2016 4 commits
    • Nathan Bronson's avatar
      clang-format some code in preparation for real changes · 3e4e50d5
      Nathan Bronson authored
      Reviewed By: djwatson
      
      Differential Revision: D2945770
      
      fb-gh-sync-id: 9e4ee3b052b0bbb33ef796b8070edd24c6942589
      shipit-source-id: 9e4ee3b052b0bbb33ef796b8070edd24c6942589
      3e4e50d5
    • Lovro Puzar's avatar
      Add call_once, wrapper around std::call_once with a fast path · 13de7740
      Lovro Puzar authored
      Summary: std::call_once is a nice API but the current GCC implementation is slower than it needs to be.  Adding a header-only wrapper with an additional atomic bool.
      
      Reviewed By: luciang
      
      Differential Revision: D2938884
      
      fb-gh-sync-id: 5939c94fe62a1523053dcff26c880ecaec9e1150
      shipit-source-id: 5939c94fe62a1523053dcff26c880ecaec9e1150
      13de7740
    • Michael Lee's avatar
      Handle wrapvFull when IOV_MAX is not defined. · 8f7fb4fb
      Michael Lee authored
      Summary:--Perhaps this should move to Portability.h instead.  There is another instance of this in io/async/AsyncSocket.cpp.--
      
      Added `portability/SysUio.h` to handle the c-macro conversion into a usable value.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2940778
      
      fb-gh-sync-id: 897e44b430c02e5a7d826f3e8da9e4979b7b898c
      shipit-source-id: 897e44b430c02e5a7d826f3e8da9e4979b7b898c
      8f7fb4fb
    • Anirudh Ramachandran's avatar
      ECDSA async/offloading support for proxygen · 167e2483
      Anirudh Ramachandran authored
      Summary: Minor changes to AsyncSSLSocket for async crypto
      
      Reviewed By: siyengar
      
      Differential Revision: D2516765
      
      fb-gh-sync-id: 354baeb94e6f63e8d5cdf8455ff5ca49a6aa479c
      shipit-source-id: 354baeb94e6f63e8d5cdf8455ff5ca49a6aa479c
      167e2483
  7. 16 Feb, 2016 2 commits
    • Iaroslav Tverdokhlib's avatar
      Fix wrapvFull for the case when iovec* has size more than 1024 · 7488256f
      Iaroslav Tverdokhlib authored
      Summary: `folly::wrapvFull` fail if the passed in `iovec*` has more than 1024 (`IOV_MAX`) elements. In particular, it returns -1 with errno 22 [Invalid argument]. The fix is to limit maximum size of iovec* to IOV_MAX that is passed in to `readv/writev/...` in a single iteration of outer loop.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2935540
      
      fb-gh-sync-id: 6c0a073ac0b59db3d53fb4269b13ddfcc479efb1
      shipit-source-id: 6c0a073ac0b59db3d53fb4269b13ddfcc479efb1
      7488256f
    • Brett Simmers's avatar
      folly::Optional<T> should be trivially destructible when T is · e2fe17ae
      Brett Simmers authored
      Summary:There doesn't appear to be a way to use std::enable_if on a
      destructor, so conditionally select from one of two storage types.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2923902
      
      fb-gh-sync-id: 2def8d1031d70379fd84e8eb555dad9d2b4996f2
      shipit-source-id: 2def8d1031d70379fd84e8eb555dad9d2b4996f2
      e2fe17ae
  8. 15 Feb, 2016 3 commits
    • Lee Howes's avatar
      Modification to futures to remove deadlock in certain use cases for getVia(executor). · 030427e2
      Lee Howes authored
      Summary: If getVia was called on a future modified using via, getVia could deadlock if the original future was updated to a new executor and there was no callback chained after the call to via. In effect: f.via(executor).getVia(executor); deadlocks. This can be a problem if the code is hidden in a library and the precise semantics are unclear. This diff adds a test that exposes the problem and a fix by forcing waitVia to add a callback that will satisfy the new exector, ensuring that drive() has a callback to trigger once the future is satisfied.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D2906858
      
      fb-gh-sync-id: a3105079530f15d7a7d39a9381c4078665b721a7
      shipit-source-id: a3105079530f15d7a7d39a9381c4078665b721a7
      030427e2
    • Andrii Grynenko's avatar
      Unify runInMainContext for void and non-void · 753868d5
      Andrii Grynenko authored
      Summary: This also fixes a bug where exception was not re-thrown for functions returning void.
      
      Reviewed By: spalamarchuk
      
      Differential Revision: D2936887
      
      fb-gh-sync-id: 9828dec131203528c27eae874aba147168f40d0d
      shipit-source-id: 9828dec131203528c27eae874aba147168f40d0d
      753868d5
    • Michael Lee's avatar
      Make Gold more like Core. · fea483aa
      Michael Lee authored
      Summary:Adding padding for Android was wrong, instead, we should have
      used `std::aligned_storage`.
      
      Reviewed By: fugalh
      
      Differential Revision: D2934566
      
      fb-gh-sync-id: f541b89309be70791fced48f63b6b5aecc49bfbb
      shipit-source-id: f541b89309be70791fced48f63b6b5aecc49bfbb
      fea483aa
  9. 13 Feb, 2016 1 commit
    • Igor Sugak's avatar
      folly/test/json_test: fix heap-buffer-overflow in Json.PrintTo test · e9ae9a1d
      Igor Sugak authored
      Summary:Json.PrintTo test if failing with heap-buffer-overflow asan abort. The problem here comes up
      when values of `std::string` and `std::ostrstring::str()` are compared.  `std::ostrstring::str()` returns
      *non null* terminated array of `char`s. When compared with `std::string` a read from memory after
      the end that array is made. Fixing the test by replacing `std::ostrstream` with `std::ostringstream`,
      that does append `\0`.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2934352
      
      fb-gh-sync-id: c6a5d765c9951716b8a14715702cf3d940c6d723
      shipit-source-id: c6a5d765c9951716b8a14715702cf3d940c6d723
      e9ae9a1d
  10. 12 Feb, 2016 4 commits
    • Andrii Grynenko's avatar
      Convert Thrift1(2)RequestDispatcher::sendMessage() · f79de567
      Andrii Grynenko authored
      Summary:This depends on D2897095.
      
      The main goal here is to start moving onRequestSuccess and onRequestFailure calls higher in the stack.
      
      Reviewed By: mmcduff
      
      Differential Revision: D2899959
      
      fb-gh-sync-id: 4e074c2d734f88f5be56000095b892c6d47c0dcc
      shipit-source-id: 4e074c2d734f88f5be56000095b892c6d47c0dcc
      f79de567
    • Mark Isaacson's avatar
      folly copyright 2015 -> copyright 2016 · 32154268
      Mark Isaacson authored
      Summary: Update copyright for 2016
      
      Reviewed By: igorsugak
      
      Differential Revision: D2828047
      
      fb-gh-sync-id: 8638188aeb694c3ca2a615cc2bc3bc9d251e388d
      shipit-source-id: 8638188aeb694c3ca2a615cc2bc3bc9d251e388d
      32154268
    • Andrii Grynenko's avatar
      Fix Singleton/ThreadLocal destruction order crashes in buck dev builds · b0174026
      Andrii Grynenko authored
      Summary: This adds a folly::Singleton->folly::ThreadLocal dependency to make sure folly::ThreadLocal is always loaded first. Otherwise PthreadKeyUnregister singleton is created after folly::Singleton storage, even though it has higher priority.
      
      Reviewed By: andrewjcg
      
      Differential Revision: D2931170
      
      fb-gh-sync-id: 36392d76e98201f2b4416b4bbef451d6c0e8c69d
      shipit-source-id: 36392d76e98201f2b4416b4bbef451d6c0e8c69d
      b0174026
    • Drew Hoskins's avatar
      Fix MultiLevelTimeseries::getRate() · 1a7befdb
      Drew Hoskins authored
      Summary:I hit a landmine where rate() returned 0 for my tests where fewer than 60 items were added per minute.  This was because it was truncating it.  And yet, countRate() was working.
      It doesn't make sense for a rate (value accumulated per time period) to be integral.
      Folly: I changed rate() to return a double by default for folly, as was done by avg() and countRate().  Looked like a typo to me.
      Common wrapper: I changed getRate() to allow you to override and make it double, as was done by getAvg().  Defaulting to int (which is usually the value type) is a bad call IMO but it's a riskier change to change it to double, and I want to be consistent with getAvg().
      
      Reviewed By: tracelog
      
      Differential Revision: D2921061
      
      fb-gh-sync-id: 00875f2ab7963ef3ba2db475aedaf6ebd413b38f
      shipit-source-id: 00875f2ab7963ef3ba2db475aedaf6ebd413b38f
      1a7befdb
  11. 11 Feb, 2016 5 commits
    • Michael Lee's avatar
      fix -Wshadowing in String.cpp · 116d93db
      Michael Lee authored
      Summary: Didn't check this with -Wshadow when I first wrote it.
      
      Reviewed By: ranjeeth
      
      Differential Revision: D2926047
      
      fb-gh-sync-id: 8391986dee2d6b5698491bd2995f039468ec684d
      shipit-source-id: 8391986dee2d6b5698491bd2995f039468ec684d
      116d93db
    • Michael Lee's avatar
      Fix typo in StringTest · 2509b98d
      Michael Lee authored
      Summary: It should be UL instead of ULL.
      
      Reviewed By: siyengar
      
      Differential Revision: D2926005
      
      fb-gh-sync-id: d2387eb601c50d9820907c831277aa6f8e938cbb
      shipit-source-id: d2387eb601c50d9820907c831277aa6f8e938cbb
      2509b98d
    • Michael Lee's avatar
      Switch stripLeftMargin to not use boost::regex · c72f7f68
      Michael Lee authored
      Summary: Remove boost regex from `stripLeftMargin`. We can shrink some binaries by not including it in the core folly library.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2922415
      
      fb-gh-sync-id: cee89164c650706f0e5c07eed3d40500831918cd
      shipit-source-id: cee89164c650706f0e5c07eed3d40500831918cd
      c72f7f68
    • Andrii Grynenko's avatar
      Add MAX_STATIC_CONSTRUCTOR_PRIORITY to PthreadKeyUnregister · 4dc763b4
      Andrii Grynenko authored
      Summary: We want onThreadExit to be canceled as late as possible. Otherwise accessAllThreads() may crash on shutdown (because ThreadEntry is destroyed, but not removed from the global list).
      
      Reviewed By: igorsugak
      
      Differential Revision: D2923435
      
      fb-gh-sync-id: 3a8eef05844bb547c06db33cb96734df1169852a
      shipit-source-id: 3a8eef05844bb547c06db33cb96734df1169852a
      4dc763b4
    • Michael Lee's avatar
      Compensate for -m32 platforms. · 1952f6b7
      Michael Lee authored
      Summary:
      When compiled for -m32, this test fails because of the
      difference between longs.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2924626
      
      fb-gh-sync-id: 8ee863fa2b6df8519bd52d0d3144697a297780bb
      shipit-source-id: 8ee863fa2b6df8519bd52d0d3144697a297780bb
      1952f6b7
  12. 10 Feb, 2016 3 commits
    • Giuseppe Ottaviano's avatar
      Minor optimizations in ExceptionCounterLib · 8cf8897e
      Giuseppe Ottaviano authored
      Summary: Avoid allocation and logging inside the throw handler, and some other minor tweaks.
      
      Reviewed By: bort, luciang
      
      Differential Revision: D2921073
      
      fb-gh-sync-id: 4491ab2f85a4251e59ba9394bba5abef0bdf7a10
      shipit-source-id: 4491ab2f85a4251e59ba9394bba5abef0bdf7a10
      8cf8897e
    • Andrii Grynenko's avatar
      Fix EventBase destruction race in FiberManagerMap · d7c3a477
      Andrii Grynenko authored
      Summary:
      Previously we could be reading from thread-local FiberManagerMap while it was modified.
      This is now fixed by keeping a per-thread list of EventBases which need to be removed from local maps. On the fast-path no action is taken, since list will be empty.
      
      This is second try, since D2853921 got reverted.
      The new implementation is simpler and does not rely on AtomicLinkedList.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D2908018
      
      fb-gh-sync-id: 4d7aed974c19761f7e2732ddbf8694af57c69bd6
      shipit-source-id: 4d7aed974c19761f7e2732ddbf8694af57c69bd6
      d7c3a477
    • Yedidya Feldblum's avatar
      stripLeftMargin · b0a7bdf6
      Yedidya Feldblum authored
      Summary:
      [Folly] `stripLeftMargin`.
      
      So you can do:
      
          TEST(MyClass, doSomethingWithString) {
            //  Multiline string literal is indented properly inside the test case
            //  instead of being aligned all the way to the left, which would make
            //  the test case read poorly.
            auto input = folly::stripLeftMargin(R"TEXT(
              first line
              second line
              third line
            )TEXT");
            auto expected = //...
            auto actual = MyClass::doSomethingWithString(input);
            EXPECT_EQ(expected, actual);
          }
      
      Reviewed By: markisaa
      
      Differential Revision: D2909736
      
      fb-gh-sync-id: ebb07da05e1a788535064cfcd9e07f617a007800
      shipit-source-id: ebb07da05e1a788535064cfcd9e07f617a007800
      b0a7bdf6
  13. 09 Feb, 2016 1 commit
    • Alan Frindell's avatar
      Relax HHWheelTimer::destroy assertion to accommodate SharedPtr · 5e5cf95a
      Alan Frindell authored
      Summary:
      HHWheelTimer's auto-pointers are kind of funny.  You can do something like this:
      
      ```
      HHWheelTimer::UniquePtr p = ...;
      // create a SharedPtr from UniquePtr
      HHWheelTimer::SharedPtr s(p);
      // create another SharedPtr from raw ptr
      HHWheelTimer::SharedPtr s(p.get());
      // No problem.
      
      If you do this:
      
      HHWheelTimer::SharedPtr p = ....;
      // this leaks
      ```
      
      Why?  Because SharedPtr is only have of std::shared_ptr.  It's the refcounting half.  But when the last SharedPtr goes out of scope, it **does not** invoke HHWheelTimer::destroy().
      
      So code like this is possible/expected:
      
      ```
      MySweetObj::MySweetObj(HHWheelTimer::SharedPtr s) {
        s_ = s;
        s_.scheduleTimeout(a, b);
      }
      
      {
        HHWheelTimer::UniquePtr p = ...;
      
        obj = MySweetObj(p)
      
        // But what if I decide to kill p:
        p.reset();
      }
      ```
      
      Since MySweetObj takes a SharedPtr and holds it, it can reasonbly expect that it can schedule timeouts on it, and the HHWheelTimer will not be deleted until it releases the SharedPtr.  This is true, but the above code would hit the assert that count_ == 0.
      
      Instead, relase the check that count_ == 0 only if there are no extra outstanding SharedPtrs.
      
      Reviewed By: viswanathgs, chadparry
      
      Differential Revision: D2908729
      
      fb-gh-sync-id: 9abd4a7d692fe952c5514dbb8d85dfbad95a3cac
      shipit-source-id: 9abd4a7d692fe952c5514dbb8d85dfbad95a3cac
      5e5cf95a
  14. 08 Feb, 2016 2 commits
  15. 07 Feb, 2016 1 commit
    • Sven Over's avatar
      folly/futures: fix early release of non-embedded callbacks · dda657cc
      Sven Over authored
      Summary:
      folly::Future (more precisely folly::detail::Core) can store
      callback functors (typically lambdas) up to a certain size
      (8*sizeof(void*)) inside the main object. Only bigger functors
      are stored in the std::function object in folly::detail::Core,
      which will put them on the heap.
      
      The behaviour of folly::Future is slightly different depending
      on whether the callback can be embedded in the main object
      or not. Small functors will be destructed after the callback
      is executed. Functors too big to fit in the lambda space in
      the Core object will be deleted when Core is deleted.
      
      Some code assumes that functor objects are deleted as soon
      as the callback has been executed. The implementations of
      folly::Future::collect and variants do so. If you switch
      off this optimisation temporarily (which can be done easily
      by setting lambdaBufSize in folly/futures/detail/Core.h to
      0), a number of unit tests fail.
      
      Given that the lambda buffer is reasonably large, most
      functors can be stored in the Core object. The different
      behaviour for very large lambdas may not have been observed.
      
      This diff fixes the inconsitent behaviour.
      
      Firstly, it changes the unit test Future:finish to explicitly
      check that the callback functor is deleted after the callback
      has been executed. This should be tested, because this
      behaviour is assumed in other parts of the futures
      implementation (e.g. Future::collect et al.).
      
      Secondly, it adds another unit test Future:finishBigLambda,
      similar to Future:finish. The lambda captures much more data
      to make sure the lambda won't be stored in the Core object,
      but in the std::function object inside Core. The test verifies
      that the behaviour is the same, i.e. the callback functor
      is destructed after the callback has been executed.
      
      Thirdly, it fixes Core and makes sure that functors of any
      size are destructued after the callback has been called. The
      new unit test fails without this.
      
      Reviewed By: fugalh
      
      Differential Revision: D2883772
      
      fb-gh-sync-id: 21a410f6592b3e090772a7b55bef6729d8739922
      dda657cc