1. 30 Sep, 2016 1 commit
  2. 29 Sep, 2016 5 commits
    • Lucian Grijincu's avatar
      folly: Range: detail::throwOutOfRange -> std::__throw_out_of_range · 10ff2d21
      Lucian Grijincu authored
      Summary:
      No need to define our own.
      
      Generated code is similar: https://godbolt.org/g/5xWrNx (one extra move instruction).
      
      ```
      .L20:
              subq    $8, %rsp
              call    S::outOfRange() [clone .isra.0]
      ```
      
      ```
      .LC1:
              .string "out of range"
      .L26:
              subq    $8, %rsp
              movl    $.LC1, %edi
              call    std::__throw_out_of_range(char const*)
      ```
      
      Reviewed By: Orvid
      
      Differential Revision: D3945578
      
      fbshipit-source-id: c65e9dea55e8f01f51766b2695af68d2bc92c266
      10ff2d21
    • Dan Schatzberg's avatar
      Fix ThreadCachedInt race condition · 6039ee41
      Dan Schatzberg authored
      Summary:
      Acquire a SharedMutex at ThreadExit to ensure that after unlinking the ThreadEntry from
      the list, future accessAllThreads() won't miss a destroying thread.
      
      This is quite a dangerous fix as it changes some lock ordering semantics. ThreadLocal
      elements are now destroyed while holding a lock, so if the destruction function
      acquires a different lock, ordering must be consistent with other
      uses of accessAllThreads().
      
      I've made accessAllThreads() an opt-in feature via a template parameter and changed
      all existing uses. I've also fixed a few lock ordering issues that arose due to this
      change.
      
      Reviewed By: andriigrynenko
      
      Differential Revision: D3931072
      
      fbshipit-source-id: 4d464408713184080079698df453b95873bb1a6c
      6039ee41
    • Subodh Iyengar's avatar
      Fix apple bug around TFO writes · 546ea578
      Subodh Iyengar authored
      Summary:
      When using connectx to do TFO, apple has a bug
      where the second write after a TFO write will cause
      the socket to throw an ENOTCONN error instead of an
      EAGAIN. Linux handles this case fine and returns an
      EAGAIN, however apple returns ENOTCONN.
      
      We solve this by treating ENOTCONN as an EAGAIN temporarily.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3942681
      
      fbshipit-source-id: ab4f0b5fd6cdcfe9c584ea00849705a2d739d65f
      546ea578
    • Subodh Iyengar's avatar
      Add tfo functions for apple · edbb8ae9
      Subodh Iyengar authored
      Summary:
      Adds TFO functions for apple devices
      Also allows android as well by removing that
      restriction. Newer versions of android support
      TFO, so this allows us to experiment with them
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3942664
      
      fbshipit-source-id: faf439783b018cf7c987a2e3ade5ea6c0c02bf48
      edbb8ae9
    • Lucian Grijincu's avatar
      folly: Range: outline exception throwing · d6c0ba01
      Lucian Grijincu authored
      Summary:
      Here's some godbolt: https://godbolt.org/g/9K36Km
      
        advanceNoInline(S):
                movq    %rdi, %rax
                sarq    $32, %rax
                cmpl    %eax, %edi
                jg      .L20
                leal    1(%rdi), %eax
                ret
        .L20:
                subq    $8, %rsp
                call    S::outOfRange() [clone .isra.0]
      
      vs previous implementation
      
        advance(S):
                movq    %rdi, %rdx
                sarq    $32, %rdx
                cmpl    %edx, %edi
                jg      .L14
                leal    1(%rdi), %eax
                ret
        .L14:
                pushq   %rbp
                pushq   %rbx
                movl    $16, %edi
                subq    $8, %rsp
                call    __cxa_allocate_exception
                movl    $.LC0, %esi
                movq    %rax, %rdi
                movq    %rax, %rbx
                call    std::out_of_range::out_of_range(char const*)
                movl    std::out_of_range::~out_of_range(), %edx
                movl    typeinfo for std::out_of_range, %esi
                movq    %rbx, %rdi
                call    __cxa_throw
                movq    %rax, %rbp
                movq    %rbx, %rdi
                call    __cxa_free_exception
                movq    %rbp, %rdi
                call    _Unwind_Resume
      
      Reviewed By: ot
      
      Differential Revision: D3940968
      
      fbshipit-source-id: b47a41e7cdd863fcef099ff3c21860b2979ee6e8
      d6c0ba01
  3. 28 Sep, 2016 4 commits
    • Giuseppe Ottaviano's avatar
      Add unchecked versions of advance(), subtract(), and subpiece() in Range · 998f6331
      Giuseppe Ottaviano authored
      Summary:
      `Range` has a somewhat inconsistent API: most methods assume
      argument validity as a precondition (mirroring their STL
      counterparts), others check the arguments and throw for invalid ones.
      
      Since `Range` is intended as a zero-cost abstraction on top of
      pointers/iterators, unchecked methods should be preferred. At this
      point however we cannot change the semantics of `advance()` and other
      methods. This diff adds new unchecked versions of these methods.
      
      Reviewed By: luciang
      
      Differential Revision: D3938480
      
      fbshipit-source-id: 6952683ee0716aa1584e79584158fbf3e083b52e
      998f6331
    • Witchakorn Kamolpornwijit's avatar
      Fix SUPPLY_MISSING_INT128_TRAITS · c85f38c3
      Witchakorn Kamolpornwijit authored
      Summary:
      The original code `AC_DEFINE([FOLLY_SUPPLY_MISSING_INT128_TRAITS...` resulted in defining the constant FOLLY_FOLLY_SUPPLY_MISSING_INT128_TRAITS in folly-config.h. This patch fix the name of the constant so that the name match with what appear in Traits.h.
      Closes https://github.com/facebook/folly/pull/485
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3931349
      
      Pulled By: Orvid
      
      fbshipit-source-id: bd7e7d3580d02134d36706bd5836822135232d26
      c85f38c3
    • Maged Michael's avatar
      folly/experimental/hazptr: fix gcc 5 build · d6e6f8b4
      Maged Michael authored
      Summary: Fix includes in memory_resource.h
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3935848
      
      fbshipit-source-id: 713ce85ad17072779dfa1c667459e367c5e1d4b7
      d6e6f8b4
    • Michael Lee's avatar
      Set a default value for slot in SharedMutex.h · c532329a
      Michael Lee authored
      Summary:
      `SharedMutexImpl::lockSharedImpl` has a potentially uninitialized access:
      
        Assume state = 0
        canAlreadyDefer = (state & kMayDefer) != 0 ==> false
        aboveDeferThreshold = (state & kHasS) >= (kNumSharedToStartDeferring - 1) * kIncrHasS ==> false
      
        if (canAlreadyDefer || (aboveDeferThreshold && !drainInProgress)) ==> false
      
        line:1452: gotSlot(slot)->compare_exchange_strong(...) uses slot uninitialized
      
      Reviewed By: Orvid
      
      Differential Revision: D3933638
      
      fbshipit-source-id: 0fbce5c00b8b1f34e50c302cb88def97853c5afe
      c532329a
  4. 27 Sep, 2016 1 commit
    • Sven Over's avatar
      Introducing folly::FunctionRef · 4b39d461
      Sven Over authored
      Summary:
      This commit introduces a simple function reference type, similar to
      std::reference_wrapper, but the template parameter is the function
      signature type rather than the type of the referenced object.
      
      A folly::FunctionRef is cheap to construct as it contains only a
      pointer to the referenced callable and a pointer to a function which
      invokes the callable.
      
      The user of FunctionRef must be aware of the reference semantics:
      storing a copy of a FunctionRef is potentially dangerous and should
      be avoided unless the referenced object definitely outlives the
      FunctionRef object. Thus any function that accepts a FunctionRef
      parameter should only use it to invoke the referenced function and
      not store a copy of it. Knowing that FunctionRef itself has reference
      semantics, it is generally okay to use it to reference lambdas that
      capture by reference.
      
      Reviewed By: ericniebler
      
      Differential Revision: D3277364
      
      fbshipit-source-id: 0a7676919cd240da5b6e1f94cadba4289e0aca28
      4b39d461
  5. 26 Sep, 2016 3 commits
    • Anirudh Ramachandran's avatar
      SSL_SESSION wrapper · 4450b4ac
      Anirudh Ramachandran authored
      Summary:
      This is a start to wrapping various SSL objects going forward so different
      binaries can choose different version of OpenSSL (i.e., BoringSSL, OpenSSL
      1.1.0, OpenSSL 1.0.2, etc.). There's no change to the caller - everyone just
      uses 'SSLSession', but the implementation details vary
      
      Reviewed By: siyengar
      
      Differential Revision: D3707791
      
      fbshipit-source-id: f895334a768cb7d43b41af40c9bc06be5307cc7f
      4450b4ac
    • Anirudh Ramachandran's avatar
      AsyncSSLSocket::getSSLClientCiphers using static map · 67dac89c
      Anirudh Ramachandran authored
      Summary:
      OpenSSL SSL_METHOD->get_cipher_by_char is not present in either OpenSSL
      1.1.0 or BoringSSL. In addition, knekritz reports that time's being spent in
      binary searching for cipher names.
      
      Since the ciphercodes and names are (fairly) static, we store these in a static
      hash map.
      
      Reviewed By: siyengar
      
      Differential Revision: D3275185
      
      fbshipit-source-id: 08b36f3e73239b415b74c6ecc30ed65832d9ebd0
      67dac89c
    • Phil Willoughby's avatar
      Make a SharedPromise from a Future · 8bed0626
      Phil Willoughby authored
      Summary: Makes it easy to split a Future into multiple Futures
      
      Reviewed By: rongrong
      
      Differential Revision: D3885897
      
      fbshipit-source-id: 6ac9fb22444dd828fbdebb44b06bf3d93d0f7583
      8bed0626
  6. 23 Sep, 2016 2 commits
    • Eli Pozniansky's avatar
      Improve documentation of MPMCQueue size and sizeGuess methods · b16d3a25
      Eli Pozniansky authored
      Summary: See title.
      
      Reviewed By: nbronson
      
      Differential Revision: D3914188
      
      fbshipit-source-id: dd9ccd0c48911632d229ae675cc40d835ea14724
      b16d3a25
    • Alisson Gusatti Azzolini's avatar
      Avoid external linkage call inside __ifunc__ · 40eaf20c
      Alisson Gusatti Azzolini authored
      Summary:
      In debug mode, CpuId() ends up triggering external linkage from inside this __ifunc__.
      However, in PIC, the relocation of external symbols are not ready yet, causing a segfault.
      
      This only reproes in dynamic linking (PIC / so) and dbg mode.
      
      Reviewed By: luciang
      
      Differential Revision: D3895239
      
      fbshipit-source-id: 2b7856c10abb5cfe24736d5bfac28e7e9d0e8272
      40eaf20c
  7. 22 Sep, 2016 5 commits
    • Michael Lee's avatar
      Switch SocketAddressTest to use TemporaryFile instead of mkstemp · c27a6839
      Michael Lee authored
      Summary: Switch from mkstemp to the slightly more portable `folly::test::TemporaryFile`
      
      Reviewed By: jsedgwick
      
      Differential Revision: D3890411
      
      fbshipit-source-id: e98d1e3a5adae92af1bb36f6213b194f633fab0f
      c27a6839
    • Giuseppe Ottaviano's avatar
      Fix a couple comments · 7c05f8af
      Giuseppe Ottaviano authored
      Reviewed By: nbronson
      
      Differential Revision: D3905865
      
      fbshipit-source-id: 2743af4ae1b34adb0f8e611e672f9b6068430ec9
      7c05f8af
    • Tianjiao Yin's avatar
      fix flaky TimeKeeper unit-test · c1ad77a5
      Tianjiao Yin authored
      Summary:
      Sometime we have such unit-test failure
      
      ```
      folly/futures/test/TimekeeperTest.cpp:134: Failure
      Value of: flag
        Actual: false
      Expected: true
      ```
      
      This diff should make it less flaky, though I am not sure what's the best way to fix this flaky unit-test.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3889630
      
      fbshipit-source-id: e7486d75fbcb3081d06724d213d4a2cf8942955d
      c1ad77a5
    • Subodh Iyengar's avatar
      Add logs for TFO succeded · 2062a6ba
      Subodh Iyengar authored
      Summary:
      Add logs for TFO to AsyncSocket
      and wangle
      
      Reviewed By: knekritz
      
      Differential Revision: D3906104
      
      fbshipit-source-id: 9a79b6f91273f8017a5e0d72fe5bdc8eff645ebc
      2062a6ba
    • Michael Lee's avatar
      experimental/observer/detail/ObserverManager.cpp missing gflags · 2bb804ca
      Michael Lee authored
      Summary: Fix the build due to a missing gflags header
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3905525
      
      fbshipit-source-id: cc09189196a04741162c2d1561545d3fec417e5e
      2bb804ca
  8. 21 Sep, 2016 1 commit
    • Shoaib Meenai's avatar
      Fix compilation for libc++ 3.9 · 60d9f847
      Shoaib Meenai authored
      Summary:
      libc++ 3.9 and above define the __throw* functions inside stdexcept, so
      defining them ourselves leads to compilation errors when compiling
      against libc++ 3.9. Add appropriate preprocessor guards to avoid this.
      
      Reviewed By: meyering
      
      Differential Revision: D3898284
      
      fbshipit-source-id: 435a28c2b3a83ee4d8f5af0df0343c524469011e
      60d9f847
  9. 20 Sep, 2016 6 commits
    • Subodh Iyengar's avatar
      Fix detaching from evb · 992ac3ae
      Subodh Iyengar authored
      Summary:
      When we attach and detach from the
      event base threads during connections
      in the case of SSL it could result in
      a crash while using TFO.
      
      When TFO was enabled connectTimeout_
      was set, which was not detached.
      
      In the case when we would
      fall back from TFO in case TFO did not
      succeed, we would try and schedule a
      connect timeout again. However because
      we are now scheduled in a new evb thread,
      this would cause the scheduleTimeout to
      assert that it was not in the correct evb
      thread.
      
      This fixes the issue by attaching and detaching
      connect timeouts as well.
      
      Reviewed By: ngoyal
      
      Differential Revision: D3892490
      
      fbshipit-source-id: 278c0b8029022144cd59366ceb0ce83f0a60a307
      992ac3ae
    • Sven Over's avatar
      use folly::Function<void()> in folly::Executor interface · 9ff27587
      Sven Over authored
      Summary:
      This diff changes the definition of `folly::Func` from `std::function<void()>`
      over to `folly::Function<void()>`. This mostly affects the interface of
      `folly::Executor` and derived and related classes. By using
      `folly::Function<void()>`, we allow to have lambdas capturing move-only types
      passed to executors. This continues an effort to get rid of the potentially
      dangerous `folly::MoveWrapper` by allowing to capture move-only types in lambdas
      when using `folly::Future`, `folly::EventBase` and now `folly::Executor`.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3706449
      
      fbshipit-source-id: 11c2cee32cb9f0298c39b7b1695a477777eeb3aa
      9ff27587
    • Maged Michael's avatar
      Draft prototype of hazard pointers C++ template library · 6a5f4337
      Maged Michael authored
      Summary: Make draft of hazard pointers prototype public
      
      Reviewed By: djwatson
      
      Differential Revision: D3870280
      
      fbshipit-source-id: e029efa336585055f67687059e10ae11766f8d7f
      6a5f4337
    • Andrey Ignatov's avatar
      Remove boost::barrier from AsyncUDPSocketTest. · c821d129
      Andrey Ignatov authored
      Summary:
      `EventBase` has its own methods to wait till it's ready so we can avoid using
      barriers and remove boost dependency.
      
      Reviewed By: mzlee
      
      Differential Revision: D3894408
      
      fbshipit-source-id: f050a982c98c4f672cf295845115686c95fc7919
      c821d129
    • Igor Sugak's avatar
      Fix fibers asan integration for new asan APIs · 691cd11c
      Igor Sugak authored
      Summary:
      Updating fibers code to use https://github.com/llvm-mirror/compiler-rt/commit/b0477747dfa8a9706f2c902e877e616aca51e06f
      
      Patch by andriigrynenko!
      
      Reviewed By: andrewcox
      
      Differential Revision: D3500482
      
      fbshipit-source-id: f51f4bb4ebf0d95a898eb1d4098459aa691acd61
      691cd11c
    • Philip Pronin's avatar
      fix ZSTD support · 151e22b2
      Philip Pronin authored
      Summary:
      Existing logic is broken (unable to correctly handle chained `IOBuf`
      in case of both `compress` and `uncompress`) and has unnecessarly strict
      `needsUncompressedLength() == true` requirement.
      
      This diff switches `ZSTDCodec` to use streaming to handle chained `IOBuf`,
      drops `needsUncompressedLength() == true`.
      
      Reviewed By: luciang
      
      Differential Revision: D3827579
      
      fbshipit-source-id: 0ef6a9ea664ef585d0e181bff6ca17166b28efc2
      151e22b2
  10. 19 Sep, 2016 1 commit
    • Qi Wang's avatar
      Try using the last Deferred reader slot first · 8d329050
      Qi Wang authored
      Summary:
      When trying to find an empty deferred reader slot, getting the current CPU can
      take quite a few cycles, e.g. >1% CPU on SMC (https://fburl.com/434646643).
      
      Let's track the last slot used by this thread and try that slot first before reading
      CPU id and doing the search.
      
      u-benchmark results seem to be improving generally (though a bit noisy and not
      sure how much to trust). Results w/ this diff on left side:
      P56648675
      
      Reviewed By: nbronson
      
      Differential Revision: D3857793
      
      fbshipit-source-id: 8b1c005362c82e748a663100f889b0b99dc257fe
      8d329050
  11. 16 Sep, 2016 6 commits
    • Dominik Gabi's avatar
      move `shellQuote` to implementation file · 5bebf3c9
      Dominik Gabi authored
      Summary: Fixing ODR violations...
      
      Reviewed By: simpkins, yfeldblum
      
      Differential Revision: D3875667
      
      fbshipit-source-id: 0d8ec0b48e14fffb7e3e60c0e68e2576b2f58d1e
      5bebf3c9
    • Dominik Gabi's avatar
      escape `{` in cmd for deprecated `Substring(std::string...)` · d9749c81
      Dominik Gabi authored
      Summary:
      Since `shellify` interprets the command as a format string we need to escape
      `{` and `}`.
      
      Reviewed By: ldemailly, simpkins
      
      Differential Revision: D3874605
      
      fbshipit-source-id: f47db387c3a44a3ba1c0c1d4726b7212fcb5ef3e
      d9749c81
    • Adam Simpkins's avatar
      update stats APIs to use TimePoint vs Duration correctly · 42ca5613
      Adam Simpkins authored
      Summary:
      Update the stats APIs to correcly distinguish between TimePoint and Duration
      types.
      
      This does leave addValue() and update() APIs in place that accept Duration
      values, for backwards compatibility.  These should eventually be removed once
      all code has been converted to call the new APIs.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3808805
      
      fbshipit-source-id: 36d6574ba4a09db7eb9f1a35e47addd3e07f8461
      42ca5613
    • Tom Jackson's avatar
      Return rvalue references from &&-qualified members of dynamic · 864eb2a6
      Tom Jackson authored
      Summary: Let the caller do a move, don't force one.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3873129
      
      fbshipit-source-id: 40c6bf564bcbf794830c99ea1248a9c1bb30e9b0
      864eb2a6
    • Vladimir Slaykovskiy's avatar
      Reduce footprint of ScribeClient · 2adb6eb0
      Vladimir Slaykovskiy authored
      Summary:
      I've found all code paths that contributed to the number of threads in new ScribeClient. Here's a list of flags that reduce the number of thread:
        --default_tls_thread_count=1 # was 32
        --sr2_event_base_pool_size=1
        --has_thrift_dispatcher_reporter=0 # creates a thrift server with a few extra threads
        --observer_manager_pool_size=1 # default is 4
      
      Optimizations in this diff:
      - Don't initialize OBCClient object if it is not used
      - Added FLAG_observer_manager_pool_size to control size of the pool in ObserverManager
      
      Currently OBC counters are switched off globally in ScribeCliean, it means that new code path that creates SR instances is disabled, but eventually we plan to switch it on. Clients that don't use SR and want to keep their RSS/threads profile low, should pass flags listed above. I'll announce this in ScribeUsers group as well.
      
      Differential Revision: D3870704
      
      fbshipit-source-id: 0efad6b3dc43c072ab11cac7e9461c09532ea11c
      2adb6eb0
    • Eric Niebler's avatar
      work around LLVM#30305 in folly::Expected, use unified initialization syntax for extra goodness · a610249b
      Eric Niebler authored
      Summary: The behavior of inheriting constructors changed in clang 3.9 (see https://llvm.org/bugs/show_bug.cgi?id=30305), and folly::Expected needs to change accordingly. As a drive-by improvement, change all invocations of default constructors to use unified initialization syntax.
      
      Reviewed By: igorsugak
      
      Differential Revision: D3872994
      
      fbshipit-source-id: fdaea8b35980df21b8522e2c3d5a8c3be1d84efa
      a610249b
  12. 15 Sep, 2016 5 commits
    • Jim Meyering's avatar
      folly/.../ExceptionTracerLib.cpp: provide less DOF fodder (avoid heap use-after-free) · 101b2ad7
      Jim Meyering authored
      Summary:
      Before this change, an application would fail consistently with a shutdown-time heap use-after-free (i.e., destructor-order-fiasco when one of the following symbols was used after its static-destruction-triggered free):
      
        DECLARE_CALLBACK(CxaThrow);
        DECLARE_CALLBACK(CxaBeginCatch);
        DECLARE_CALLBACK(CxaRethrow);
        DECLARE_CALLBACK(CxaEndCatch);
        DECLARE_CALLBACK(RethrowException);
      
      Each of those would define a classic meyers singleton.
      Since each destructor is trivial, we can fix this by making each a pseudo-leaky (indestructible) meyers singleton instead. Since each static value is never destroyed,
      it can never cause a DOF error again.
      
      Differential Revision: D3870740
      
      fbshipit-source-id: 625f5d5268768ca0e4125bed72bc66c53618be29
      101b2ad7
    • Christopher Dykes's avatar
      Kill unneeded dependency on portability/Memory.h · 6848287d
      Christopher Dykes authored
      Summary: This dependency was added but isn't actually needed, so get rid of it.
      
      Reviewed By: mzlee
      
      Differential Revision: D3863923
      
      fbshipit-source-id: 407d2b3db759e30c37d876dde133fc962daeaeae
      6848287d
    • Neel Goyal's avatar
      Add flag to indicate a session resumption attempt · fd032019
      Neel Goyal authored
      Summary: Track when session resumption was attempted during connect
      
      Reviewed By: shamdor-fb
      
      Differential Revision: D3861028
      
      fbshipit-source-id: 26ca41084faeeb64666b6833896b9510ef2b4c25
      fd032019
    • Adam Simpkins's avatar
      a simple first step towards using clocks properly in the stats code · 8b030a19
      Adam Simpkins authored
      Summary:
      Update the timeseries and histogram classes to accept a clock parameter as a
      template parameter, instead of a time duration type.
      
      This is a first step towards transitioning the code to correctly distinguishing
      between time_point and duration types.  This defines TimePoint and Duration
      type aliases, but does not start using them yet.
      
      In upcoming diffs I will start converting more APIs to correctly use TimePoint
      instead of just Duration.
      
      For now the default clock type is folly::LegacyStatsClock, which still uses
      std::chrono::seconds as the default duration.  At the moment the stats code is
      optimized for second granularity--the addValue() code has a fast path when
      called in the same second as the last update.  When using finer granularity
      durations this fast path can't be used as often.  I will also send out
      subsequent diffs to make the code optimized for updates within the same bucket,
      rather than just updates with the exact same time value.
      
      Reviewed By: yfeldblum
      
      Differential Revision: D3807715
      
      fbshipit-source-id: 77696c4f44a8d85e4d6ff84d7656fe7a9709797c
      8b030a19
    • Dominik Gabi's avatar
      ignore `$SHELL` in `shellify` · 9637519e
      Dominik Gabi authored
      Summary:
      Why `$SHELL` is a bad idea:
      
      - getenv() is not thread safe. (In practice it should work if other threads aren't calling setenv(), but still seems undesirable if we can avoid it.)
      - It seems confusing for the program to have different behavior for different developers.
      - Other shells besides /bin/sh may have different quoting behaviors. For instance, I don't think csh allows unescaped newlines inside single quotes.
      - SHELL might be set to other non-shell-like programs in some cases. (Say, if this gets run from inside a git or mercurial hook it might end up being set to the restricted git or hg shell that only lets you run specific commands.)
      Anyway, this isn't related to your diff, so nothing needs to be done for now,
      but I would vote for changing this to always use /bin/sh in a future diff.
      
      Both the C system() call and python's subprocess module appear to
      always use /bin/sh and ignore $SHELL.
      
      Reviewed By: simpkins
      
      Differential Revision: D3867047
      
      fbshipit-source-id: dab0e6afbe1c20ff13d9a52f212df95af425dd77
      9637519e