1. 26 Jun, 2014 7 commits
    • Hans Fugal's avatar
      Scheduler interface of Executor · d7a43326
      Hans Fugal authored
      Summary: and ManualExecutor implementation
      
      Test Plan: unit tests, contbuild
      
      Reviewed By: davejwatson@fb.com
      
      Subscribers: bmatheny, folly@lists, net-systems@, fugalh, exa, marccelani, jsedgwick
      
      FB internal diff: D1392999
      
      Tasks: 4548494
      d7a43326
    • Tudor Bosman's avatar
      folly OSS fixes: add ThreadName.h and compression · 0ecc7473
      Tudor Bosman authored
      Summary: Also, optionalize dependencies on compression libraries.
      
      Test Plan: fbconfig -r folly && fbmake runtests_opt
      
      Reviewed By: meyering@fb.com
      
      Subscribers: kma, jhj, simpkins, lesha, folly@lists
      
      FB internal diff: D1396573
      0ecc7473
    • Tudor Bosman's avatar
      Make randomNumberSeed read from /dev/urandom · 6019aaaa
      Tudor Bosman authored
      Summary: Because @lesha asked "why not" and I couldn't give him an answer.
      
      Test Plan: random_test
      
      Reviewed By: bmaurer@fb.com
      
      Subscribers: bmaurer, folly@lists, jhj, kma, lesha, sdoroshenko, soren
      
      FB internal diff: D1394401
      6019aaaa
    • Peter Ruibal's avatar
      Add ThreadName.h to folly's Makefile.am · 418ad481
      Peter Ruibal authored
      Summary:
      fbthrift depends on <folly/ThreadName.h>, which isn't currently
      getting installed as part of the autotools build.  Add it to Makefile.am
      
      Test Plan:
      Made this change to folly, re-autogen/configure/install, and
      then was able to successfully compile fbthrift's compiler
      
      Reviewed By: davejwatson@fb.com
      
      Subscribers: doug, folly@lists
      
      FB internal diff: D1397084
      418ad481
    • Yunqi Zhang's avatar
      Expose EVLOOP_NONBLOCK · 51e98fa7
      Yunqi Zhang authored
      Summary:
      This diff allows users to loop through EventBase without blocking if there are
      not any events to process.
      
      This is useful for sending and receiving requests on network, where users just
      want to try if there are any events and do not want to block if not.
      
      https://phabricator.fb.com/D1373887 is an example where we find this feature
      useful, otherwise we have to add an empty callback before loop.
      event_base_.runInLoop([] {});
      event_base_.loopOnce();
      
      @davejwatson, @fugalh, @simpkins, @stepan: Could you please take a look at the
      proposed changes and let me know if there is any better ways of doing this.
      
      Thank you!
      
      Test Plan:
      I think this would not break anything, but we might want to do some performance
      profiling if needed.
      
      Reviewed By: hans@fb.com
      
      Subscribers: simpkins, davejwatson, fugalh, stepan, folly@lists
      
      FB internal diff: D1383401
      51e98fa7
    • Nicholas Ormrod's avatar
      Make fbstring libgcc-safe · a97b3be5
      Nicholas Ormrod authored
      Summary:
      Some libgcc-incompatible code has been added to fbstring.
      Removed/reorganized it so that we can drop fbstring right into libgcc.
      
      Test Plan:
      fbconfig -r folly && fbmake runtests
      
      Copied FBString.h into libgcc's basic_fbstring.h, with no modifications.
      Successfully tp2_build libgcc/4.8.1. Adjusted symlink, then fbmake clean
      && fbconfig -r folly && fbmake dbg. The fbmake dbg failed with an
      assertion error, which is consistent with @lucian's observations in
      D1373725; the important part is that the error was at runtime, so the
      compile-time changes of this diff looks good.
      
      Reviewed By: lucian@fb.com
      
      Subscribers: folly@lists, sdwilsh, njormrod, lucian
      
      FB internal diff: D1382873
      a97b3be5
    • Nicholas Ormrod's avatar
      FBString conservative additions · 58d27d00
      Nicholas Ormrod authored
      Summary:
      Now that fbstring is conservative by default (D1373308), we can remove
      the mutability of the data members and the call to c_str() in operator[].
      
      Test Plan: fbconfig -r folly && fbmake runtests
      
      Reviewed By: lucian@fb.com
      
      Subscribers: folly@lists, sdwilsh, njormrod
      
      FB internal diff: D1382644
      58d27d00
  2. 14 Jun, 2014 5 commits
    • Anton Likhtarov's avatar
      Fix open source build · 1a31dc64
      Anton Likhtarov authored
      Test Plan: build it
      
      Reviewed By: pavlo@fb.com
      
      Subscribers: folly@lists
      
      FB internal diff: D1383722
      1a31dc64
    • Lucian Grijincu's avatar
      folly: fbstring: make it conservative-only: write '\0' in ctor and drop the c_str shenanigans · 94045182
      Lucian Grijincu authored
      Test Plan: ran folly tests
      
      Reviewed By: njormrod@fb.com
      
      Subscribers: folly@lists, njormrod
      
      FB internal diff: D1373308
      94045182
    • Nicholas Ormrod's avatar
      fbstring conservative corruption · 7e726e2a
      Nicholas Ormrod authored
      Summary:
      @lucian's D1373308 set fbstring to conservative by default.
      This breaks, eg, ti/proxygen/httpclient tests, by failing an assertion
      inside of c_str(). Specifically, the terminator is not '\0'.
      
      The fbstring_core move constructor, when sourced by a MediumLarge
      string, steals the source's internal data, then resets the source by
      calling ##setSmallSize(0)##. That function sets the in-situ size of the
      fbstring to zero, thus requalifying the string as a small string;
      however, it does nothing to the data - the previous 23 bytes now contain
      garbage.
      
      Sources of a move must be in a consistent state after the move is
      complete. The source, once a MediumLarge string whose first eight bytes
      were a pointer, is now a small string of size zero whose first byte is
      not necessarily '\0'. This breaks the FBSTRING_CONSERVATIVE invariant.
      
      This can be fixed by writing a terminator after the setSmallSize call.
      I have fixed all setSmallSize locations that do not writeTerminator.
      
      fbstring_core's move constructor is called exclusively from
      basic_fbstring's move assignment operator, hence the odd format of the
      test case.
      
      == TMI ==
      
      Interestingly, the source will almost certainly* contain a '\0', which
      prevents this simple ##str.size() != strlen(str.c_str())## bug from
      turning into a memory-trampling monster. The new size of zero is not
      what saves us - the 'size' byte of a small fbstring, through a very
      clever trick to allow 23-byte in-situ strings, is actually 23 minus the
      actual size (now 0), so is 23! Where, then, does the '\0' byte come? A
      MediumLarge string's data layout is [pointer, size, capacity]. The
      pointer is not guaranteed to contain a '\0', and neither are size or
      capacity. However, the size of the string needs to be very large in
      order to force the top byte of the size member to be non-zero; in that
      case, the string is so large that malloc is returning memory by the
      page. Since page sizes are a multiple of 2^8 (almost always, and if not
      then I don't think your fbstring can support large enough sizes
      anyways), and we use goodMallocSize, the capacity pointer would have a
      least signfigicant byte of zero.
      
      Why the (*)? Well, when reserving extra space on a non-refcounted Large
      string, the reallocation does not yield its extra goodMallocSize space.
      This could be fixed, though probably isn't worth the trouble. Anyways,
      since we aren't goodMallocSize-ing the user-supplied requested capacity,
      it probably won't contain a '\0'.
      
      Test Plan:
      fbconfig -r folly && fbmake runtests
      
      Modify folly/test/FBStringTest.cpp to define FBSTRING_CONSERVATIVE, then
      fbconfig folly/test/:fbstring_test_using_jemalloc && fbmake runtests
      Note that this fails before the patch is applied.
      
      Note that it is possible for the tests to pass even when this bug is
      present, since the top byte of the heap pointer must be non-0 in order
      for this error to be triggered.
      
      Reviewed By: lucian@fb.com
      
      Subscribers: folly@lists, njormrod, lucian, markisaa, robbert, sdwilsh, tudorb, jdelong
      
      FB internal diff: D1376517
      7e726e2a
    • Nicholas Ormrod's avatar
      static-ify FBString asserts · d6a1e277
      Nicholas Ormrod authored
      Summary: Some asserts could be static_asserts. Make it so!
      
      Test Plan: fbconfig -r folly && fbmke opt && fbmake runtests_opt
      
      Reviewed By: lucian@fb.com
      
      Subscribers: folly@lists, sdwilsh, njormrod
      
      FB internal diff: D1378670
      d6a1e277
    • Vojin Katic's avatar
      folly::gen::splitByLine · 9cc86052
      Vojin Katic authored
      Summary:
      I made it work, but please send your feedback how to improve code quality.
      
      splitByLine will split on \r, \n, and \r\n.
      
      Test Plan: add new test, arc unit
      
      Reviewed By: tjackson@fb.com
      
      Subscribers: folly@lists, crawler-diffs@
      
      FB internal diff: D1322212
      9cc86052
  3. 09 Jun, 2014 19 commits
    • Jim Meyering's avatar
      folly: do not disable RW_SPINLOCK_USE_X86_INTRINSIC_ for clang · cd5e0d07
      Jim Meyering authored
      Summary:
      Without this, we'd see problems like this in tao, when building with clang:
      With this change, this now works with clang-3.4 and clang.dev (3.4+).
      This change reverts D950285, which change appears to have been made
      to accommodate weakness in clang-3.3 or older.
      
      In file included from tao/data_providers/common/simpledp.cpp:7:
      ./tao/data_providers/common/stats.h:175:18: error: no type named 'RWTicketSpinLockT' in namespace 'folly'
      typedef folly::RWTicketSpinLockT<64, true> RWLockType;
      ~~~~~~~^
      ./tao/data_providers/common/stats.h:175:35: error: expected member name or ';' after declaration specifiers
      typedef folly::RWTicketSpinLockT<64, true> RWLockType;
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
      
      Test Plan:
      ensure that all of these pass:
      fbconfig -r --clang folly/test:rw_spinlock_test && fbmake runtests
      fbconfig -r --clang folly/test:rw_spinlock_test && fbmake runtests_opt
      fbconfig -r         folly/test:rw_spinlock_test && fbmake runtests_opt
      fbconfig -r --clang --with-project-version clang:dev \
      folly/test:rw_spinlock_test && fbmake runtests_opt
      
      Reviewed By: delong.j@fb.com
      
      Subscribers: folly@lists, mathieubaudet
      
      FB internal diff: D1370024
      
      Tasks: 4090011
      cd5e0d07
    • Zejun Wu's avatar
      Specialize string to identical string conversion · 587427e7
      Zejun Wu authored
      Summary:
      Avoid copying underlying char array in to<string>(const string&) and
      to<fbstring>(const fbstring&).
      
      Test Plan: fbconfig -r $redteamisthebestteam/$nekomikoreimu && fbmake
      
      Reviewed By: ldbrandy@fb.com
      
      Subscribers: jonp, folly@lists
      
      FB internal diff: D1368183
      
      Tasks: 4263125
      587427e7
    • Marcus Holland-Moritz's avatar
      Add support for returning number of benchmark iterations · aed6ef6f
      Marcus Holland-Moritz authored
      Summary:
      I'm looping through a large number of test cases in a benchmark and I'm
      interested in the average time per test case rather than the total time,
      which is what I'm currently seeing. In order to get the average time,
      I need to be able to tell the benchmark module how many iterations have
      been run.
      
      This change adds _MULTI variants of the different BENCHMARK_ macros that
      allow for returning the actual number of iterations that have been run,
      e.g.:
      
      BENCHMARK_MULTI(benchmarkSomething) {
      std::vector<int> testCases { 0, 1, 1, 2, 3, 5 };
      for (int c : testCases) {
      doSomething(c);
      }
      return testCases.size();
      }
      
      Test Plan:
      * fbconfig -r folly && fbmake runtests
      * added new test cases to example benchmark code
      
      Reviewed By: simpkins@fb.com
      
      Subscribers: folly@lists, london_search@
      
      FB internal diff: D1356437
      aed6ef6f
    • Philip Pronin's avatar
      revert "conditionally write terminator in c_str()" · ec08f7d4
      Philip Pronin authored
      Summary: D1318048#21
      
      @override-unit-failures
      
      Test Plan: fbconfig -r folly && fbmake runtests_opt -j32
      
      Reviewed By: njormrod@fb.com
      
      Subscribers: folly@lists, njormrod
      
      FB internal diff: D1368939
      
      Tasks: 4466412
      
      Blame Revision: D1318048
      ec08f7d4
    • Stepan Palamarchuk's avatar
      Introduce destruction callbacks · d816ed88
      Stepan Palamarchuk authored
      Summary:
      This change allows users to track lifetime of EventBase and perform clean shutdown when EventBase gets destructed.
      
      It is useful for users that rely on EventBase lifetime, but don't have any feedback mechanism with the owner of EventBase.
      
      For instance some part of code might remain running in background on the EventBase after the main object was destroyed (e.g. it might be finalizing some async requests). In such case the original owner doesn't know that there's something still running and may try to destroy EventBase. In that case such background code will remain zombie forever.
      
      AsyncMcClient changes are presented just as an example of usage.
      
      @davejwatson, @simpkins: Could you please take a look at the proposed changes for the EventBase? If this is something not worth adding into EventBase, could you recommend a better way of doing things?
      
      Test Plan: fbmake runtests
      
      Reviewed By: alikhtarov@fb.com
      
      Subscribers: folly@lists, simpkins, davejwatson
      
      FB internal diff: D1353101
      d816ed88
    • Marcelo Juchem's avatar
      Allowing additional arguments to be passed to split_step's functor · 3ed4f961
      Marcelo Juchem authored
      Summary: more flexibility for using functors with split_step
      
      Test Plan: unit tests added + arc unit
      
      Reviewed By: ldbrandy@fb.com
      
      Subscribers: folly@lists
      
      FB internal diff: D1362644
      3ed4f961
    • Tudor Bosman's avatar
      Hasher and equality comparison for IOBuf · 867b291c
      Tudor Bosman authored
      Test Plan: test added
      
      Reviewed By: davejwatson@fb.com
      
      Subscribers: folly@lists
      
      FB internal diff: D1359469
      867b291c
    • Matt Dordal's avatar
      Timed wait for futures · a91af7b3
      Matt Dordal authored
      Summary:
      It might be useful to be able to wait for some time (but not forever) on a
      future, so this is a shot at doing that. It's a very heavyweight implementation, however.
      
      Since the current interface for waitWithSemaphore doesn't really make sense if
      the timeout fires, change it to return a Future<T>.
      
      Test Plan: unit tests
      
      Reviewed By: hans@fb.com
      
      Subscribers: trunkagent, folly@lists, fugalh
      
      FB internal diff: D1358230
      a91af7b3
    • Jon Purdy's avatar
      Add missing make_unique overload. · 19ec26e1
      Jon Purdy authored
      Summary: C++14 adds this overload but I wanted it today.
      
      Test Plan: It compiles, and this is the definition described in the standard.
      
      Reviewed By: xning@fb.com
      
      Subscribers: folly@lists
      
      FB internal diff: D1338839
      19ec26e1
    • Philip Pronin's avatar
      use NoInt() wrappers in FileUtil · 300f64f9
      Philip Pronin authored
      Summary:
      Accidentally spotted this problem.  `folly/FileUtil.h` and
      `common/files/FileUtil.h` are now using `*NoInt` wrappers where appropriate.
      
      Test Plan: fbconfig -r common/files folly && fbmake opt -j32
      
      Reviewed By: lucian@fb.com
      
      Subscribers: folly@lists, fbcode-common-diffs@lists
      
      FB internal diff: D1356261
      300f64f9
    • Tom Jackson's avatar
      Minor edit to comment · c7f8903c
      Tom Jackson authored
      Summary: I was confused by it, thought rephrasing might help.
      
      Test Plan: Read
      
      Reviewed By: tudorb@fb.com
      
      Subscribers: folly@lists
      
      FB internal diff: D1315612
      c7f8903c
    • Dave Watson's avatar
      Remove extraneous syscalls if NotificationQueue size() > 1 · cf214a5b
      Dave Watson authored
      Summary:
      Currently notification queue does 2 syscalls per item: one read, one write.  We only need the eventfd to notify to wake up the thread, so instead, if the thread is already awake, don't bother writing to the fd.
      
      Benchmark shows that when the queue size() > 1, this is ~4x faster.
      
      Note that this might be unfair if there are multiple consumers: I could imagine a situation where one thread eats all the wakeups written to the fd, so only one thread is actually working.  However, multiple consumers is a bad idea anyway, and I'd consider removing it entirely:  If the same fd is in multiple epoll() loops, _all_ epolls will wake up, resulting in a thundering herd problem.  I don't see any multiConsumer cases in fbcode
      
      Using EFD_SEMAPHORE or not doesn't seem to matter, since hopefully we're only writing 1 wakeup per thread - and it wouldn't work at all for multiConsumer case.
      
      Test Plan:
      fbconfig thrift/lib/cpp/test:TNotificationQueueTest; fbamke runtests
      fbconfig common/concurrency:QueueBenchmark
      fbmake opt
      QueueBenchmark --bm_min_iters=10000
      
      Reviewed By: afrind@fb.com
      
      Subscribers: doug, folly@lists, fbcode-common-diffs@lists, alandau, bmatheny, haijunz
      
      FB internal diff: D1272872
      
      Tasks: 2802758
      cf214a5b
    • Jim Meyering's avatar
      folly/wangle: temporarily disable compilation of Thens.cpp · 5a5aee33
      Jim Meyering authored
      Summary:
      This code fails to compile with clang:dev, so don't try for now.
      * folly/wangle/test/Thens.cpp: Don't attempt to compile test/Thens.cpp.
      See 4412111 for details.  Prompted by clang:dev+MSAN effort, 4090011.
      
      Test Plan:
      Run this:
      fbconfig --clang --with-project-version clang:dev -r folly/wangle
      fbmake runtests
      Failed before, passes with this patch.
      
      Reviewed By: hans@fb.com
      
      Subscribers: folly@lists, fugalh
      
      FB internal diff: D1354751
      
      Tasks: 4090011, 4412111
      5a5aee33
    • Yedidya Feldblum's avatar
      Add shorthand functions to Format.h. · 4460753f
      Yedidya Feldblum authored
      Summary:
      [Folly] Add shorthand functions to Format.h.
      
      This makes calling code simpler when it does not depend on the raw performance of writing to a stream.
      
      Test Plan:
      $ fbconfig -r folly/test
      $ fbmake runtests
      
      Reviewed By: tudorb@fb.com
      
      Subscribers: folly@lists, dougw
      
      FB internal diff: D1347353
      
      Tasks: 4391110
      4460753f
    • Matt Dordal's avatar
      add support for whenAll to waitWithSemaphore · d82763a6
      Matt Dordal authored
      Summary:
      waitWithSemaphore currently doesn't support vector<Try<T>>, unless T is void.
      Fix that, and also add a now-required void specialization.
      
      Test Plan:
      Add a test that uses vector<Try<bool>>, ensure that the tests compile
      (and pass).
      
      Reviewed By: hans@fb.com
      
      Subscribers: folly@lists, fugalh
      
      FB internal diff: D1338528
      
      Tasks: 4389473
      d82763a6
    • Simon Martin's avatar
      Future::value() should throw when unset · 3bb51070
      Simon Martin authored
      Summary:
      Added a test to call Future::value() before the Promise value is set, expecting an exception.
      In a dbg build the test failed due on the assertion in Optional::value().
      In a opt build the test failed due as no exception was thrown.
      There are 2 points where we could throw our exception:
      a) Optional::value() - replacing the assertion
      b) Future::value()
      
      I'm not sure which location makes the most sense.
      With the assertion in Optional it seems that adding the throw here would not be unexpected but this is outside the wangle code.
      So as a first pass I've added the throw in Future::value(), and made a new WangleException for this.
      
      Test Plan:
      $ fbconfig folly/wangle
      $ fbmake runtests
      
      Reviewed By: hans@fb.com
      
      Subscribers: folly@lists, fugalh
      
      FB internal diff: D1340886
      3bb51070
    • Daniil Burdakov's avatar
      added missing includes; also fixed lint issue with noexcept · 206c8f8f
      Daniil Burdakov authored
      Summary: subj
      
      Test Plan: unit tests
      
      Reviewed By: tjackson@fb.com
      
      Subscribers: folly@lists
      
      FB internal diff: D1341693
      206c8f8f
    • Tudor Bosman's avatar
      More opensource build fixes · edf578d8
      Tudor Bosman authored
      Summary:
      - libtool version
      - get rid of tiny libraries
      - add folly/gen and a bunch of stuff from experimental
      
      Test Plan: built, built a program against it in a ubuntu vm
      
      Reviewed By: davejwatson@fb.com
      
      Subscribers: folly@lists, fugalh
      
      FB internal diff: D1339920
      edf578d8
    • Adam Simpkins's avatar
      make BucketedTimeSeries::addValue() honor old timestamps · 253dd087
      Adam Simpkins authored
      Summary:
      Previously BucketedTimeSeries()::addValue() documented that it required
      time to move forwards.  If it was ever called with a timestamp older
      than the most recent one it had seen, it would just use latestTime_ as
      the time, and add the value to the most recent bucket.
      
      This changes addValue() so that it always uses the timestamp passed in
      by the caller.  If this time value refers to an old bucket that is still
      being tracked, the data point will be added to that bucket.  If the time
      value is older than the oldest currently tracked bucket, the data point
      will be ignored, and false will be returned.
      
      I did consider leaving the current addValue() behavior as-is, and
      requiring a separate addHistoricalValue() for when users intentionally
      want to try adding old data points.  However, it seems nicer to build
      this into the existing addValue() function.  The old behavior of just
      replacing the supplied time value seems potentially surprising to users.
      
      This does change the behavior of addValue(), and therefore could affect
      the behavior of some programs.  However, up until now no-one should have
      been calling addValue() with old time values, as it wouldn't have done
      what they want anyway.  I did a brief search through our code base, and
      all call sites I saw always called addValue() with the current time.
      (Most of the callers use wall clock time, so this change might affect
      program behavior if the system time changes after the program starts.
      We should ideally change our programs to use monotonic clocks instead.)
      
      Test Plan:
      Included a new unit test.
      
      Also compared the timeseries_benchmark results before and after this
      change.  Overall this new logic seems to be faster.  For the "all time"
      case, the new code is over 2x as fast.  For the normal, non-all-time
      case the new code is around 5% faster.
      
      Reviewed By: hans@fb.com
      
      Subscribers: doug, folly@lists, net-systems@, exa
      
      FB internal diff: D1338466
      253dd087
  4. 20 May, 2014 9 commits
    • Tudor Bosman's avatar
      Some opensource build fixes · 527ce886
      Tudor Bosman authored
      Summary:
      - switch to new versions of ax_boost_*.m4
      - versioning in libtool
      - better checks in configure.ac
      
      Test Plan: built in an Ubuntu VM
      
      Reviewed By: davejwatson@fb.com
      
      Subscribers: folly@lists
      
      FB internal diff: D1338957
      527ce886
    • Tudor Bosman's avatar
      Build up signal handler message before writing · 2ff7b237
      Tudor Bosman authored
      Summary: So it doesn't interleave with whatever other threads write to stderr.
      
      Test Plan: folly/experimental/symbolizer/test
      
      Reviewed By: lucian@fb.com
      
      Subscribers: folly@lists
      
      FB internal diff: D1337029
      2ff7b237
    • Matt Dordal's avatar
      fix waitWithSemaphore return type · f327d89e
      Matt Dordal authored
      Summary:
      waitWithSemaphore always tried to return a value, which is not what the
      underlying implementation did. If the value_type was an object, it would
      fail to compile.
      
      Test Plan: unit tests (added one to compile all the variants)
      
      Reviewed By: hans@fb.com
      
      Subscribers: folly@lists, fugalh
      
      FB internal diff: D1326916
      f327d89e
    • Alex Landau's avatar
      Make EventHandler::isPending const · ae442842
      Alex Landau authored
      Summary: Because it just queries state
      
      Test Plan: fbmake
      
      Reviewed By: haijunz@fb.com
      
      Subscribers: folly@lists
      
      FB internal diff: D1332397
      ae442842
    • Nicholas Ormrod's avatar
      Removed old FBVector compatibility functions · 2728ed9f
      Nicholas Ormrod authored
      Summary: FBVector still has some code for gcc-4.6. Removed it.
      
      Test Plan:
      fbconfig -r folly && fbmake runtests
      fbconfig folly/test/stl_test && fbmake runtests (after enabling)
      
      Reviewed By: robbert@fb.com
      
      Subscribers: folly@lists, sdwilsh
      
      FB internal diff: D1320358
      2728ed9f
    • Rocky Liu's avatar
      Revert "[folly::Subprocess] Set O_CLOEXEC by default when creating pipes to... · cd209b34
      Rocky Liu authored
      Revert "[folly::Subprocess] Set O_CLOEXEC by default when creating pipes to avoid race conditions resulting from concurrent Subprocess creations"
      
      Summary: This reverts commit c2f089cf080f2b3effa9efa5e4708b9674437d45.
      
      Test Plan: Compile && folly::Subprocess unit tests
      
      Reviewed By: tudorb@fb.com
      
      FB internal diff: D1327952
      
      cd209b34
    • Rocky Liu's avatar
      Always #define _GNU_SOURCE to pull in pipe2() declarations · f05ff6b8
      Rocky Liu authored
      Summary: [folly::Subprocess] Always #define _GNU_SOURCE to pull in pipe2() declarations
      
      Test Plan: Compile
      
      Reviewed By: tudorb@fb.com
      
      FB internal diff: D1327004
      f05ff6b8
    • Akshay Vaidya's avatar
      Adding a release function for ThreadLocalPtr. · b51f8cd7
      Akshay Vaidya authored
      Summary:
      ThreadLocalPtr manages the lifecycle of the object that is
      stored with it. We have a use case where we sometimes want to transfer ownership
      of the stored object to another thread by wrapping them with
      unique_ptrs. Adding a release function, similar to to the
      unique_ptr::release is the cleanest way for us to transfer ownership.
      
      Test Plan:
      I can do some on off testing using a command line tool, but I
      was wondering about how to add some unit tests. Not sure when the folly
      unit tests were.
      
      Reviewed By: njormrod@fb.com
      
      FB internal diff: D1321588
      b51f8cd7
    • Rocky Liu's avatar
      Set O_CLOEXEC by default when creating pipes to avoid race conditions... · 870912b8
      Rocky Liu authored
      Set O_CLOEXEC by default when creating pipes to avoid race conditions resulting from concurrent Subprocess creations
      
      Summary:
      [folly::Subprocess] Set O_CLOEXEC by default when creating pipes to avoid race conditions resulting from concurrent Subprocess creations
      
      If multiple threads are creating Subprocess objects concurrently, the
      write side file descriptor of the pipe created in the parent process
      might be inherited into other child processes unintentionally and never
      closed, causing the parent process to hang while reading from the read
      side of its pipe, thinking the other side must have been closed.
      
      The fix to the problem is to create the pipes and set O_CLOEXEC in
      a single pipe2 call. Then the child could clear the O_CLOEXEC flag
      selectively before calling exec().
      
      Test Plan:
      Existing unit tests of Subprocess
      Added a new unit test which will hang in Subprocess constructor without
      this fix.
      
      Reviewed By: tudorb@fb.com
      
      FB internal diff: D1267396
      870912b8