1. 27 Jul, 2015 5 commits
    • Sophia Wang's avatar
      fix clang ASAN error in DelayedDestructionBaseTest · 8fe31d96
      Sophia Wang authored
      Summary: fix a use after free bug. this line is stupid!
      
      Reviewed By: @siyengar
      
      Differential Revision: D2283041
      8fe31d96
    • James Sedgwick's avatar
      delete folly/wangle · 8e922526
      James Sedgwick authored
      Summary: she's gooooooone, oh why, oh why, i better learn how to face it...
      
      https://www.youtube.com/watch?v=bnVXIUyshng
      
      Reviewed By: @djwatson
      
      Differential Revision: D2219135
      8e922526
    • Sara Golemon's avatar
      Bump version to 52:0 · eafa57ba
      Sara Golemon authored
      eafa57ba
    • Alexey Spiridonov's avatar
      Make ProcessReturnCode default-constructible · 3f277b37
      Alexey Spiridonov authored
      Summary: We have this previously-unused "NOT STARTED" status, which I recently appropriated to denote moved-out `ProcessReturnCode`s.
      
      It's natural to also use this for default-constructed `ProcessReturnCodes`. Lacking a default constructor leads to a bunch of unnecessarily annoying use of `folly::Optional` in my upcoming diff, so I wanted to get rid of that, see e.g.
      
      differential/diff/7657906/
      
      Reviewed By: @tudor
      
      Differential Revision: D2097368
      3f277b37
    • Alexey Spiridonov's avatar
      Improve waitpid error handling · b10e3e86
      Alexey Spiridonov authored
      Summary: Using `checkUnixError` after `waitpid()` is confusing and useless, because the only two possible errors are `ECHILD` (some other part of the program waited for this process!?) and `EINVAL` (invalid options, which are hardcoded in both `Subprocess::wait()` and `poll()`). Neither of these are recoverable. Moreover, even if the caller catches the exception, `~Subprocess` is still booby-trapped to `abort()` since its status remains `RUNNING`.
      
      In short, just abort.
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D2079415
      b10e3e86
  2. 25 Jul, 2015 1 commit
    • mwilliams's avatar
      Fix some warnings in folly · 7b481d96
      mwilliams authored
      Summary: Remove a couple of unused variables, and move one
      that was only used inside an ifdef
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D2279989
      7b481d96
  3. 24 Jul, 2015 5 commits
    • Nathan Bronson's avatar
      added a missing hook to IndexedMemPool's testing harness · 30e9f34e
      Nathan Bronson authored
      Summary: IndexedMemPool had one bare usage of std::atomic, rather than
      the templated type Atom.  This doesn't affect any non-testing template
      instantiations, because those two are usually synonyms, but it could cause
      spurious failures of DeterministicSchedule tests.  Found via inspection,
      not via failed tests.
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D2277424
      30e9f34e
    • Misha Shneerson's avatar
      Print type of singleton that is requested after destruction · 29d3553c
      Misha Shneerson authored
      Summary: As titled
      Trying to troubleshoot shutdown problems in HHVM and I see:
        EventBase::runAfterDelay() callback threw St13runtime_error exception: Raw pointer to a singleton requested after its destruction.
      Would be much easier to troubleshoot if I know what is it being requested.
      
      Reviewed By: @chipturner
      
      Differential Revision: D2274497
      29d3553c
    • Nathan Bronson's avatar
      improvements to DeterministicSchedule · a4fc31fc
      Nathan Bronson authored
      Summary: This diff isolates the CacheLocality back end for DeterministicSchedule
      from one invocation to the next (the last one was deterministic
      across the entire program but not per-DeterministicSchedule instance),
      and makes it easy to do extensive tracing of memory accesses during
      a deterministic test.  These changes were made while tracking down a
      bug, but don't fix any bugs on their own (and in fact don't affect any
      production code at all).
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D1842390
      a4fc31fc
    • Andre Pinto's avatar
      Fix oss build · 2f0b7834
      Andre Pinto authored
      Summary: Add program_options to fix oss build
      
      Reviewed By: @tudor
      
      Differential Revision: D2274863
      2f0b7834
    • Nick Terrell's avatar
      Delete functions that return a pointer when the dynamic object is a rvalue. · 8e48b79d
      Nick Terrell authored
      Summary: This diff is not yet complete, I want to see the contbuild before I change the
      functions that return references to member functions.
      
      It is unsafe to return a pointer when the dynamic object is a rvalue, because if
      the pointer escapes the expression after the object is destroyed, we go into
      segfault / undefined behavior land.
      I have deleted these overloads.  The amount of valid code that is now disallowed
      is minimal.  The only valid case I can think of is returing a pointer and
      passing it to a function in the same expression that does not save the pointer.
      However, this case is also dangerous, because if the function you pass it to
      decides to save the pointer for later, we are in trouble, e.g.
      
        save_ptr(dynamic("str").c_str())
      
      Since there are simple workarounds (naming the object), I think that is a small
      price to pay for the greatly increased safety.
      
      The next step is to overload all members that return a reference to a member
      to move the member out if the dynamic is a rvalue:
      
        const dynamic& at(dynamic const&) const&;
              dynamic& at(dynamic const&)      &;
              dynamic  at(dynamic const&)      &&;  // Move out
      
      I also need to go over the code more carefully to make sure that nothing went
      wrong.
      
      Reviewed By: @marcinpe
      
      Differential Revision: D2257914
      8e48b79d
  4. 23 Jul, 2015 2 commits
    • Tudor Bosman's avatar
      Helper for writing nested command line apps · 5532f19f
      Tudor Bosman authored
      Summary: Many command line apps are of the form
      "program [--global_options] command [--command_options] args..."
      
      Make writing such things less painful.
      
      +jdelong because smcc
      
      Reviewed By: @meyering
      
      Differential Revision: D2217248
      5532f19f
    • Tudor Bosman's avatar
      Make gflags and boost::program_options play nice with each other · 838cf146
      Tudor Bosman authored
      Summary: GFlags is useful for global program options. boost::program_options makes it
      easier to write command-line utilities. They don't, unfortunately, play
      very nicely with each other.
      
      Add a addGFlags() function to convert all GFlags to boost::program_options
      options; you can then use boost::program_options::parse_command_line() to
      parse all arguments, GFlags or not.
      
      Also add a helper function to make parsing nested command lines easier.
      
      Reviewed By: @fugalh
      
      Differential Revision: D2215285
      838cf146
  5. 22 Jul, 2015 4 commits
    • Nathan Bronson's avatar
      fix racy assert in SharedMutex · 7334598f
      Nathan Bronson authored
      Summary: SharedMutex purposely allows the inline reader count to underflow in some
      situations while preserving proper locking behavior (the inline reader
      count is only trusted if all deferred locks have been applied), but there
      was an additional way that this could occur that wasn't documented or
      allowed by the asserts.  The effect was a false positive assert in rare
      conditions or the possibility of losing track of a deferred lock slot.
      This diff fixes both the over-aggressive assert and the potential loss
      of the shared slot.  If the assert didn't fire for you then this diff
      won't change the correctness of your program.
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D2269018
      7334598f
    • Tudor Bosman's avatar
      Add executable_path() and convert SubprocessTest to use it · cd464069
      Tudor Bosman authored
      Summary: Because I didn't want to write it again.
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D2217246
      cd464069
    • Ondrej Lehecka's avatar
      making thrift and folly header files compile clean with -Wunused-parameter · 89282758
      Ondrej Lehecka authored
      Summary: fixing compiler errors when compiling with Wunused-parameter
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D2267014
      89282758
    • Hannes Roth's avatar
      (Wangle) window, fix test · 4ae8db54
      Hannes Roth authored
      Summary: Thanks @​peijinz for finding this.
      
      Reviewed By: @​peijinz
      
      Differential Revision: D2268532
      4ae8db54
  6. 21 Jul, 2015 7 commits
  7. 20 Jul, 2015 11 commits
    • Sara Golemon's avatar
      Bump version to 51:0 · 8cf17bb5
      Sara Golemon authored
      8cf17bb5
    • Brian Watling's avatar
      Allow access to a FiberManager's currently running fiber · 7aff2d80
      Brian Watling authored
      Summary: Allow access to a FiberManager's currently running fiber
      
      Reviewed By: @alikhtarov, @andriigrynenko
      
      Differential Revision: D2257201
      7aff2d80
    • Nick Terrell's avatar
      Overload Optional::value() on object reference type. · 1470961d
      Nick Terrell authored
      Summary: `folly::Optional` returns the stored value by reference when the object
      is an rvalue.  This causes three issues:
      
        * If the user calls `value()` on an rvalue `Optional`, and assigns the result
          to a new variable, the copy constructor gets called, instead of the move
          constructor.  This causes the added test `value_move` to not compile.
        * If the user calls `value()` on an rvalue `Optional`, and assigns the result
          to a const lvalue reference, they might expect the lifetime to be extended
          when it isn't. See the added test `value_life_extention`.
        * Assigning the results of `value()` on an rvalue `Optional` to a mutable
          lvalue reference compiled in the old code, when it shouldn't, because that
          is always a dangling reference as far as I can tell.
      
      I'm not sure how strict `folly` is with compatibility, but I believe the
      breakage would be minimal, and any code that gets broken probably deserves it.
      
      I'm not exactly sure who I should add as a reviewer, so hopefully herald has got
      my back.
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D2249548
      1470961d
    • Steve O'Brien's avatar
      folly Singleton: clear some state if creator function fails · ad7e7f72
      Steve O'Brien authored
      Summary: The creator thread ID is saved to indicate the singleton is already being built (to help detect dependency cycles).  However if the creation function throws an error, that thread ID persists, and then if the same thread tries again to build the singleton it will be falsely detected as a dependency cycle.  This clears that state in the case of failure.
      
      Reviewed By: @chipturner
      
      Differential Revision: D2256441
      ad7e7f72
    • Hannes Roth's avatar
      Revert: (Wangle) within should raise TimedOut() · 8a876701
      Hannes Roth authored
      Summary: This reverts commit 956351018a495af89575526536af7e7c0bb285aa.
      
      Reviewed By: @​labrams
      
      Differential Revision: D2258219
      8a876701
    • Steve O'Brien's avatar
      folly HHWheelTimer: fix loop variable · 489a7f09
      Steve O'Brien authored
      Summary: In nested loop, loop condition is incorrect.  Fixed var (should be `ii` not `i`)
      
      Reviewed By: @pgriess
      
      Differential Revision: D2255702
      489a7f09
    • Tom Jackson's avatar
      Making Optional throw exceptions instead of assert · 22e8caf3
      Tom Jackson authored
      Summary: I'm upgrading assertions to throws, since these are fatal in all circumstances. If something is explicitly `Optional`, it makes sense to fail loudly if it is misused in this manner.
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D2247612
      22e8caf3
    • James Sedgwick's avatar
      add SocketPair.h/.cpp to Makefile · 59bdbbcc
      James Sedgwick authored
      Summary: to unbreak OSS wangle tests build
      
      Reviewed By: @bugok
      
      Differential Revision: D2240264
      59bdbbcc
    • Tudor Bosman's avatar
      folly::Future-istic barrier · 83312f92
      Tudor Bosman authored
      Summary: What it says on the tin.
      
      Reviewed By: @fugalh
      
      Differential Revision: D2230390
      83312f92
    • Pavlo Kushnir's avatar
      Add an option to disable guard pages for fiber stacks · 84f796c9
      Pavlo Kushnir authored
      Summary: guard pages may make debugging more painful. For example, in some cases they increased "perf" initialization time.
      
      Reviewed By: @alikhtarov
      
      Differential Revision: D2247188
      84f796c9
    • Hannes Roth's avatar
      (Wangle) within should raise TimedOut() · 65fd92b6
      Hannes Roth authored
      Summary: I figured it out. This works. The two extra futures are a small overhead
      (just two pointers). The `Core`s are allocated anyway, so this is pretty
      much optimal.
      
      A timeout now raises on the current Future, and a fulfilled promise
      cancels the timeout Future.
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D2232463
      65fd92b6
  8. 15 Jul, 2015 5 commits
    • Sara Golemon's avatar
      Only try to use F_SETPIPE_SZ if it's available · c23480d0
      Sara Golemon authored
      Summary: As the comment says, we can ignore errors in setting the size.
      So it stands to reason we can ignore setting the size as well.
      
      Reviewed By: @yfeldblum
      
      Differential Revision: D2242882
      c23480d0
    • Sophia Wang's avatar
      Make DestructorGuard inherit from a base · f46045c9
      Sophia Wang authored
      Summary: There are more use cases that the Destruction/Guard pattern can be
      used than current DelayedDestruction provides. This diff makes the pattern more
      general (remove self destruct) and lets DelayedDestruction derive from that.
      The functionalities of DelayedDestruction is kept unchanged.
      
      I leave destroy(), Destructor class, and destroyPending_ in DelayedDestruction
      since they are not required by our CallbackGuard in proxygen.
      
      I add a shouldDestruct() function to allow customized conditions on when to
      call destructor.
      
      I haven't made destroyNow() a std::function since I only need it to be set at
      instatiation time. If there is any other use case that needs destroyNow() to be
      a std::function, I can do that as well.
      
      Reviewed By: @afrind
      
      Differential Revision: D2202575
      f46045c9
    • Joe Richey's avatar
      Actually denote when we have infinite generators · a1107271
      Joe Richey authored
      Summary: Before we didn't do anything at all with the ::infinite value for our
      generators, now all the sources operators and sinks are properly notated. The
      one signifcant change regarding what is allowed concerns 'cycle' which now (when
      called with no arguments) always produces an infinite generator. This is fine
      for all but the weirdest of generators fed into cycle.
      
      Reviewed By: @ddrcoder
      
      Differential Revision: D2240328
      a1107271
    • Joe Richey's avatar
      Changing behavior of 'any' and 'all' sinks, adding in 'isEmpty' and 'notEmpty' sinks · 43d53e06
      Joe Richey authored
      Summary: When adding in the 'filter()' default behavior, I considered adding in similar
      behavior for 'any' and 'all'. However, we had 'any' with no funciton call
      basically check if anything was present, not testing a predicate. This can
      create a confusing senario, so I removed this behavior from 'any' and added in
      the 'isEmpty' and 'notEmpty' sinks. Now the calls 'any()' and 'all()' (called
      with parens, so old uses won't compile) check for truthy values simlar to
      'filter()'.
      
      I also added some unit tests and changed 'static const' objects to 'constexpr'.
      
      Reviewed By: @ddrcoder
      
      Differential Revision: D2234637
      43d53e06
    • Andrii Grynenko's avatar
      RequestContext support · cc671d29
      Andrii Grynenko authored
      Summary: Integrating RequestContext into fibers library. RequestContext is saved for every task, when that task is created. If RequestContext is overriden when a task is being run (within fiber, in runInMainContext, within function passed to await call) the new context will continue to be used for the task.
      
      Reviewed By: @alikhtarov
      
      Differential Revision: D2240152
      cc671d29