Summary: We don't support Python 2 anymore.

Reviewed By: xavierd

Differential Revision: D33715602

fbshipit-source-id: 6cf3dba83f8207f956cab8eb8dbb3a748e1d9f89

Summary: The future is now.

Reviewed By: xavierd

Differential Revision: D33714537

fbshipit-source-id: 8d282bbe7391c4b72b70dab54a5c252060fba457

Summary: Declared maxReadAtOnce_ in AtomicNotificationQueue.h as uint_32 to avoid implicit conversion in AtomicNotificationQueue-inl.h

Reviewed By: yfeldblum

Differential Revision: D33645415

fbshipit-source-id: 0eb57074951f1320df2dbb1d302cba69a727af56

Summary:
There was a bug in the logic where negative 18-digit numbers would be incorrectly assessed as not fitting int64_t even though they do.
The issue was that `integral.size() < maxIntLen` condition didn't work correctly for some negative numbers. In particular, for 18-digit negative numbers, `integral.size()` would be 19 (minus sign + 18 digits) which is not smaller than `maxIntLen` which is also 19. The subsequent check `(negative && integral.size() == minIntLen && integral <= minInt)` would also fail because `minIntLen` is 20.

Reviewed By: yfeldblum

Differential Revision: D33732185

fbshipit-source-id: 0ff70387e3b450acf45efc950a9114a13e752386

Summary:
Original commit changeset: 7d3b54d26bc3

Original Phabricator Diff: D33304127 (https://github.com/facebook/folly/commit/21329c4cb847b7cf9ce0610449df0272df945d70)

Reviewed By: yfeldblum

Differential Revision: D33733281

fbshipit-source-id: 683de43fc0d32aa40d3a6a6ef49f8b2d3760f48f

Summary: This is more efficient for some underlying transports, including Fizz.

Reviewed By: yfeldblum

Differential Revision: D33724711

fbshipit-source-id: 35ebcce83da4a9289f7f083c8873142e9b844ca0

Summary: Add writem method to MockAsyncUDPSocket.

Reviewed By: yfeldblum

Differential Revision: D33696768

fbshipit-source-id: eb99650f3dcde4eacbf83be3dd8ef9d2c03c6200

Summary: Use the new tpx option to run tests locally on the OSS CI hosts

Reviewed By: bigfootjon

Differential Revision: D33454014

fbshipit-source-id: dcf49dcebcd476c68b3eb46eb013079c30db52c7

Summary:
A refcounted-string type suitable for exception types.

Exception types should ordinarily be nothrow-copy-constructible, which excludes holding `std::string` fields, and should ordinarily be as small as possible, which excludes holding `std::shared_ptr<std::string>` fields.

Reviewed By: luciang

Differential Revision: D33010058

fbshipit-source-id: ae71f00d2053c9c52b9a980d028dce8397704cda

Summary: Add a feature to apply and read variable length socket options.

Differential Revision: D33410246

fbshipit-source-id: 3407a926c2ba9323a2318f3d34f8e1f1173bde59

Summary: Work around "incompatible pointer types assigning" errors reported by clang top-of-tree.

Reviewed By: Gownta

Differential Revision: D33482980

fbshipit-source-id: 2d7cf408c45ae96540c5c7779f8462f01d5b3498

Summary:
On a single-core Ubuntu VM, getdeps attempts to pass -j0 to Boost Jam,
which fails.

Reviewed By: fanzeyi

Differential Revision: D33675531

fbshipit-source-id: 629ee41448941213aadec10658583d8f984b13c2

Summary:
Original implementation uses minimum running time as a result even if it will be just a single iteration, which might introduce high error rate.

This diff targets to improve benchmarking stability by adding a new method that estimates running time over 1 sec, runs benchmark with multiple ecpoches about half a second each for 5 sec, drops top and bottom 25% percentile measurements to remove outliers, and gives a geomean of whats left.

This feature is not enabled by default, as it might considarably increse total running time, and can be turned on by using `-bm_estimate_time` knob.

Reviewed By: Gownta

Differential Revision: D33478923

fbshipit-source-id: 144d8e16f8279a30a1eccf1d9494d607893532c8

Summary:
Original commit changeset: 2f869768f3a8

Original Phabricator Diff: D33432903 (https://github.com/facebook/folly/commit/e7eb2bb6db6c4b8f7ecb368236c2244d07b58f91)

The prior commit broke the open source build for Watchman:

```
Exception: project openssl has no builder for <getdeps.manifest.ManifestContext object at 0x7f053d23fe80>
```

Reviewed By: lnicco

Differential Revision: D33675540

fbshipit-source-id: cdf11a967a2234263e5713eaf2959ec9dc2b0e52

Summary:
Previously by default, `Symbolizer` would always use the current process's
binary by looking at `/proc/self/exe`. This change allows passing a
different binary name to `Symbolizer` so that we can reuse this library
to look up symbol information of a different process that is executed
from a different binary.

Reviewed By: Gownta

Differential Revision: D33558166

fbshipit-source-id: 12c6ae3aa8c5d88caaef7933edb2c69bc896d75b

Summary:
Warnings are triggered by `is_pod<>` with both Clang and GCC when
using `-std=c++20`.

This change replaces all instances of `is_pod<T>::value` with
`(is_standard_layout<T>::value && is_trivial<T>::value)`, which
is equivalent and even suggested by both compilers.

Pull Request resolved: https://github.com/facebook/folly/pull/1695

Reviewed By: Gownta

Differential Revision: D33483730

Pulled By: Orvid

fbshipit-source-id: 79bd867c7e019f614c6187ca9ca5c44d65bf6310

Summary: The coroutine library has non-elidable wrappers for alloc/dealloc. They directly call `operator new` and `operator delete` but, of course, they have conditionals. Switch to the wrappers which handle the conditionals.

Reviewed By: luciang

Differential Revision: D33181403

fbshipit-source-id: ab8d56634569b82680b36d6b72fb9ff7f5c05f19

Summary:
Closes: https://github.com/facebook/folly/issues/1700.

The comparison is always false and therefore dead. Remove it.

Reviewed By: philippv

Differential Revision: D33647863

fbshipit-source-id: c882f998d1bf7d53cdba0a2c8e82a809a6b5fc13

Summary: They are replaced by `operator_new` and `operator_delete`.

Reviewed By: luciang

Differential Revision: D33180955

fbshipit-source-id: 891fc10fa53a5b53928321ba87a8d46e5953d188

Summary: Implement tryPost and include some simple tests

Reviewed By: magedm

Differential Revision: D33586443

fbshipit-source-id: 27c90ddf7e8578cb45c53e0083aa39910e5b0982

Summary: The CallbackRecord interface is unfamiliar to users and inconsistent with the general direction of folly::coro, and as a result these algorithms have not seen any usage. Deletes them and moves CallbackRecord into the only file that uses it. (We may eventually want to replace it if we come up with a story for Try<Reference> but I'm not tackling that here).

Differential Revision: D33570204

fbshipit-source-id: 9cfb0def927e5882f582d6c1eb280d97c9ff3e22

Summary:
This diffs adds
* `--shared-libs` command line argument to the `build` command which enables building shared libraries for supported projects (cmake-based projects, boost, libevent)
* this flag overrides `BUILD_SHARED_LIBS` cmake flags in manifest files and from `--extra-cmake-defines`
* adds `shared_libs=on` expression support in manifest files
* for boost, the flag enables building shared libraries **in addition to** statically linked ones

Reviewed By: simonmar

Differential Revision: D27462289

fbshipit-source-id: d22ab434f7228c30472611bc323830d88efba0a5

Summary: Based on scsiguy's feedback. Probably it can be made shorter and clearer still, feel free to propose more changes.

Reviewed By: scsiguy

Differential Revision: D33591287

fbshipit-source-id: e0bf85ebfd98d7032ddbd54c69c52b33525a62c7

Summary:
An executable built by clang-cl that is linked against folly/logging
from vcpkg will fail to link due to a few missing symbols.  folly/logging
has a number of functions that are only defined when `__INCLUDE_LEVEL__` is defined.
vcpkg always uses MSVC, MSVC does not support this macro while clang-cl does,
resulting in mismatch between what is defined, used, and declared.

These ifdefs mostly came from fc507b26.

The (minor IMO) negative consequence of this diff is that
`XlogFileScopeInfo` will be unnecessarily larger (a pointer and an uint32)
 on compilers that don't support `__INCLUDE_LEVEL__` (e.g MSVC).

The actual condition of whether to make use of `__INCLUDE_LEVEL__` remains, via the `XLOG_IS_IN_HEADER_FILE` macro.

Pull Request resolved: https://github.com/facebook/folly/pull/1617

Reviewed By: yfeldblum

Differential Revision: D29753787

Pulled By: Orvid

fbshipit-source-id: 28f8128c981b255960d7838d2df2a3d299153bc2

Summary:
Part of the added comment about `folly::coro::merge()`:

  // Currently this doesn't fully do structured concurrency. Care is needed to
  // avoid use-after-free when the stream is terminated early, as some cancelled
  // input streams may be left running in background.
  // The rule is: if output generator's next() returned an empty value
  // (end of stream), it's guaranteed that 'sources' and all input coroutines are
  // completed and destroyed, no use-after-free danger. In all other cases
  // (next() returned an exception or the output generator was destroyed early),
  // the input generators may be left running in background (cancelled and
  // detached).

This diff adds a long comment about this and a couple other quirks (or what I would consider quirks).

It also fixes one of the quirks: the merged stream may return end-of-stream when 'source' coroutine frame is not destroyed yet. So if the 'source' coroutine e.g. has a `SCOPE_EXIT` that references some state owned by the caller of `merge()`, it may in principle use-after-free. This diff destroys 'source' coroutine frame just before returning end-of-stream rather than just after. This way `merge()` is at least possible to use in "strucutured concurrency" style, as long as you avoid exceptions and always drain the merged generator before destroying it.

It would be easy to do the same for the exception case (just wait for the `makeConsumerTask()` coroutine to complete before calling `throw_exception()`). This would make `merge()` as close to structured concurrency as is possible with `folly::AsyncGenerator` (which can always be synchronously destroyed, leaving no choice but to detach child coroutines; but as long as the user doesn't do that, it should be fine). But I'm hesitant to do it because some existing call sites might rely on current behavior. I'll leave this to the team that actually owns this code. Same for the two cancellation quirks listed in the added comment.

Reviewed By: iahs

Differential Revision: D33557074

fbshipit-source-id: cf7aaee8ec78e21c370d1f7faff3b4bad1327c95

Summary:
folly::fibers uses a SIGSEGV handler to detect fiber overflows, by checking if
the faulting address resides within a guard page range.

In the case when it does not, prior to this diff, the folly::fibers signal
handler would reraise the signal and allow the previous sigsegv handler handle
the exception.

The problem with this is that reraising the signal will discard the original
`siginfo` context sent by the kernel. The old signal handler would lose
visibility on the machine state, faulting condition, etc.

This does not play nicely with folly::symbolizer. Typically,
folly::symbolizer's signal handler will print information on the faulting
signal, such as:

```
*** Signal 11 (SIGSEGV) (0xdeadbeef) received by PID 3140647 (pthread TID 0x7fe2cddf1380) (linux TID 3140647) (code: address not mapped to object), stack trace: ***
    @ 00000000002f4d3d folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
                       ./folly/experimental/symbolizer/SignalHandler.cpp:449
    @ 0000000000000000 (unknown)
    @ 00000000002dd0f9 doCrashWildRead()
                       ./scripts/mingtao/oneoffs/crash.cc:9
    @ 00000000002dd0b1 main
                       ./scripts/mingtao/oneoffs/crash.cc:47
    @ 0000000000025dc4 __libc_start_main
    @ 00000000002dcfb9 _start
                       /home/engshare/third-party2/glibc/2.30/src/glibc-2.30/csu/../sysdeps/x86_64/start.S:120
Segmentation fault (core dumped)
```

From the error message, you can see that:
* the faulting address was 0xdeadbeef
* the reason for SIGSEGV was because the application accessed a region of memory outside of its virtual address space

If you happened to have used `folly::fibers` somewhere within your application, though, you instead would see this:

```
*** Aborted at 1639532805 (Unix time, try 'date -d 1639532805') ***
*** Signal 11 (SIGSEGV) (0x1c85b0032a0f8) received by PID 3318008 (pthread TID 0x7f5573e8d380) (linux TID 3318008) (maybe from PID 3318008, UID 116827) (code: -6), stack trace: ***
    @ 00000000002f4e0d folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
                       ./folly/experimental/symbolizer/SignalHandler.cpp:449
    @ 0000000000000000 (unknown)
    @ 00000000002dd169 doCrashWildRead()
                       ./scripts/mingtao/oneoffs/crash.cc:9
    @ 00000000002dd124 main
                       ./scripts/mingtao/oneoffs/crash.cc:48
    @ 0000000000025dc4 __libc_start_main
    @ 00000000002dcfb9 _start
                       /home/engshare/third-party2/glibc/2.30/src/glibc-2.30/csu/../sysdeps/x86_64/start.S:120
```

`code: -6` corresponds to `SI_TKILL`, indicating that the signal was a result of some process invoking `tgkill` (this is the fiber handler re-raising the signal)

And `0x1c85b0032a0f8` is some bogus value, since signals raised by `tgkill` do not have a valid si_addr field.

Furthermore, when debugging the core file, `p $_siginfo` will no longer give the relevant machine state at the time of the crash, since `$_siginfo` now would correspond to the
raised signal by folly::fibers, rather than the original fault.

This diff changes the `raise` to a synchronous invocation of the old signal handler. This allows the original signal context to be properly preserved (and visible within core).

Reviewed By: Gownta

Differential Revision: D33165801

fbshipit-source-id: 4526d5a9a93cf1eba13e9f5a16b13d00b3c8c85b

Summary: Add variadic template argument parameters to async_invocable_inner_type (so it can support overloaded callables).

Differential Revision: D33534807

fbshipit-source-id: 8bd1d2bc48bb27592b9d728dbd67e85bbed32144

Summary: I don't see how this header works, as it includes folly/Portable.h (which defines FOLLY_HAS_STRING_VIEW) _after_ it has already used FOLLY_HAS_STRING_VIEW a few lines up. Somehow, that worked with buck1 (really not sure how), but broke buck2.

Reviewed By: ot

Differential Revision: D33511408

fbshipit-source-id: 9e2da2a5c25a02367079254ff78552a5a335b3b1

Reviewed By: andriigrynenko

Differential Revision: D33532498

fbshipit-source-id: bd180566563576d003afced43684a8e0cb8e3cee

Summary:
folly::Subprocess attempts to close (nearly) all file descriptors if
`closeOtherFds()` option is set. When the process limit for number of
file descriptors is large, but the actual number of open descriptors is low,
much time is spent calling `close()` on files that are not actually open.

On Linux, we can use the information in `/proc/self/fd` to close only those
files that are actually open to speed up the process in the normal case.

Reviewed By: yfeldblum

Differential Revision: D33128499

fbshipit-source-id: efa5a348ff657c8212d9329a3efc7dd5bc0bb4d9

Summary: This ensures that when we collect the normal stack traces while collecting the async stack traces, the normal stack addresses are within a valid range. The normal stack addresses may be invalid because some functions may not follow the usual calling convention, for example with TSAN or with Lua FFI calling into C++.

Reviewed By: yfeldblum

Differential Revision: D32108552

fbshipit-source-id: 97b652f5e71b67dfa810549d837c7a0393d7c4cb

Summary:
Fixes the bug referenced in the base commit of this stack.

Given that `folly::dynamic(1) == folly::dynamic(1.0)` returns true, `folly::dynamic(1) < folly::dynamic(1.0)` should return false to maintain ordering properties.

Regarding the PrintTo test change - the keys in the json map are ordered - so now the numeric ordering behavior kicks in and order is changed. If we really wanted to we could use a custom comparator for when `json::serialization_opts::sort_keys == true` but this option I'm guessing is only for easing human consumption, and json keys of float-like things are probably not common so complicating the code may not be warranted.

Reviewed By: Gownta

Differential Revision: D33465565

fbshipit-source-id: 21859a1b0e63cc8c3d0c002f3478baae1cf7fc18

Summary:
Fixes the bug referenced in the base commit of this stack.

Given that `folly::dynamic(nullptr) == folly::dynamic(nullptr)` returns true, `folly::dynamic(nullptr) < folly::dynamic(nullptr)` should return false to maintain ordering properties.

Reviewed By: Gownta

Differential Revision: D33456202

fbshipit-source-id: 3fc994cd76d6e119884e017f68b445ed438f7cea

Add tests for folly::dynamic ordering comparison operators on scalar/string types and highlight some bugs

Summary:
# Summary
Currently nullptr vs nullptr and int vs double ordering comparison operators (e.g. `operator<`) are inconsistent with their equality operators.

This diff just adds tests to increase coverage and highlight the broken areas. The subsequent diffs will address the issues individually.

The behavior on nulls at least has been present since the very beginning of when a CompareOp was added for it in 2017.

More details are below.

# Bug 1: nullptr vs nullptr
If we have
```
folly::dynamic a(nullptr);
folly::dynamic b(nullptr);

a < b // returns true
a == b // returns true
```
This violates that the two should be equal if `!(a < b) && !(b < a)` given that this expression will return `false`, whereas `a == b` returns `true`.

This property is required by e.g. std::sets.
If one puts a `folly::dynamic(nullptr)` in an std::set with the current behavior, when iterating it will be found, and will contribute to its size, but `std::set::find()` on it will return false. Hash-based sets are unaffected - given that the hash and `operator==` implementations are correct.

The fix is straightforward.
`struct dynamic::CompareOp<std::nullptr_t>` in dynamic-inl.h should return false.

# Bug  2: int vs double (hash + ordering operators)
The `folly::dynamic==` operator does numeric comparison for int64 vs double - whereas the `folly::dynamic<` operator does not - instead falling back on comparing against the Type enum value. Similar to the above this violates the principle of `operator<` being usable for checking equality - given that it is really just checking type equality - but has already been established in the code path that they're different types.

Hashing is also inconsistent given that a hash of dynamic(2.0) will not equal dynamic(2) even though they are equal, given that each uses the hash of the underlying type. When ordering is fixed, hashing should be too so that hash based and ordering based containers have the same behavior.

The fixes are straightforward.

operator< in dynamic.cpp should pull the same logic from operator== with regards int vs double.
See https://www.internalfb.com/code/fbsource/[4e70ca24b9ec]/fbcode/folly/dynamic.cpp?lines=97-126

hashing can be resolved by having doubles use integer hashing in dynamic.cpp - https://www.internalfb.com/code/fbsource/[4e70ca24b9ec]/fbcode/folly/dynamic.cpp?lines=307

 ---

Reviewed By: Gownta

Differential Revision: D33465564

fbshipit-source-id: 43b926837e20f4a9afe15ee0032d469864e360c0

Reviewed By: Gownta

Differential Revision: D33476592

fbshipit-source-id: 9e66fdf07ce88c507cfb32406b113e1c05131c8c

Summary: This opportunity was found with the combination of Infer's PULSE_UNNECESSARY_COPY and Strobelight cycle counts.

Reviewed By: Gownta

Differential Revision: D33304127

fbshipit-source-id: 7d3b54d26bc3e171cc7a0a831933fdbb9638f6a8

Summary: sizeof(SharedMutex) = 4, sizeof(std::mutex) = 40. This diff makes sure that we use smaller mutexes both in Observer Core and in Observables given that we may end up with many instances of these objects, yet there mutexes are accessed only on the update path.

Reviewed By: praihan

Differential Revision: D32271615

fbshipit-source-id: ba7d04632a677a9eb4ae398d681dadac3b06f033

Reviewed By: yfeldblum

Differential Revision: D33118848

fbshipit-source-id: c69b1f8f415b130d5d7a6487e7027d5c1e49865f

Summary: Add comments stating the expectations for Baton post and Futex wake native implementation to be async-signal-safe.

Reviewed By: ot, luciang

Differential Revision: D33461663

fbshipit-source-id: 2855427cc94f577b566b19203a4523f54a41d0aa

Summary:
A number of places were extracting dependencies from manifests, but only one was adding in the implicit dependencies for build tools.

Extract the logic to one place and use so that a change in a tool like cmake will now correctly affect all tools using cmake, as it will be taken into account as a dependency hash when the manifest's hash is computed.

Tests for this change revealed that install_dirs needed to be populated in reverse order from the manifest topo-sort, so have also addressed that

Reviewed By: wittgenst

Differential Revision: D32730717

fbshipit-source-id: 1b2a25e460de6085d274c99acfd391b3bd259264