Commit Graph

287 Commits

Author SHA1 Message Date
Wez Furlong
309fc66a2c eden: add timeout to FuseChannel requests
Summary:
The macOS FUSE implementation has the concept of a daemon timeout,
which is used to set an upper bound on the length of time that the kernel will
wait for a request to be satisfied.  If a request takes too long, the kernel
will shutdown the fuse device and the user is left with a relatively broken
experience in their repo until they restart the eden server.  Critically, when
it is left in the broken state it is hard for the user to realize that they
need to restart the server because the kernel will still respond with EIO to
file accesses within the mount; even though it has been stopped, it isn't
fully unmounted until the fuse process stops.

This diff introduces a self imposed timeout in the fuse processing flow
and emits an ETIMEDOUT error when it is reached.

The intent is that we'll configure this timeout to be smaller than the
macOS daemon_timeout (which will be adjusted in a separate diff) so
that we don't trigger the problematic behavior when the kernel decides
that we've timedout.

This change is made for all fuse implementations, not just macOS, for
the sake of consistency: there's value to doing this on Linux as well,
to avoid some deadlock like scenarios: this should put an upper bound
on blocking in certain situations.

I've made the timeout default to 60 seconds, but haven't added any
configuration code for this yet; will do that in a follow on diff.

Reviewed By: chadaustin

Differential Revision: D16917954

fbshipit-source-id: 675539c43cf7f0009fd65d138081b9126464b7e0
2019-08-28 09:41:54 -07:00
Wez Furlong
fe2a08e3e7 eden: add FUSE_NO_OPENDIR_SUPPORT to capsLabels
Summary:
on linux kernel 5.1 we noticed that some caps were requested but not
known to the diagnostic code.  This little diff adds a missing entry.

There is still one more that is unknown, but I think that we'll need to update
our copy of the fuse header to get that handled here.

Reviewed By: chadaustin

Differential Revision: D16953397

fbshipit-source-id: 30d19843a2cf1ec22fd469654773afaafa8e3b9b
2019-08-28 06:46:42 -07:00
Chad Austin
c89607f129 build fuse_tester with open source gflags
Summary: Allow building fuse_tester with open source gflags and glog.

Reviewed By: strager

Differential Revision: D16636221

fbshipit-source-id: 5128e936d5a6b6d0ed33bf34b770ee661728af3f
2019-08-21 17:22:46 -07:00
Brian Strauch
1556dec257 Total FUSE access time
Summary: Record a rolling sum of the time taken by any FUSE call on a per-process basis

Reviewed By: strager

Differential Revision: D16553149

fbshipit-source-id: 54f1e453916727a40f245b294239dc1b232a8967
2019-08-02 09:44:03 -07:00
Brian Strauch
3abcc3bef1 Record backing store imports with RequestData
Summary: Uses the existing RequestData class to make calls to static functions to set and get the `didImportFromBackingStore` flag.

Reviewed By: simpkins

Differential Revision: D16461868

fbshipit-source-id: e3ed39249f5dd1a842ad06a204b5933014b12f7f
2019-08-01 13:38:31 -07:00
Brian Strauch
d0acc0f175 Count FUSE reads/writes
Summary:
Display FUSE calls, reads, and writes

{F167451325}

Reviewed By: chadaustin

Differential Revision: D16214570

fbshipit-source-id: ce1b3533d7260fb304c7efdaef8567a83d3dcd4a
2019-07-26 10:08:10 -07:00
Chad Austin
fe64ec3874 use fb303 repo in open source build
Summary: Add a dependency from the eden open source build to the fb303 open source build and switch EdenServiceHandler to BaseService.

Reviewed By: simpkins

Differential Revision: D15528156

fbshipit-source-id: 2ca5c31dd9fcc9bac43fd399b27f33b6f2c5ebfc
2019-07-24 21:07:04 -07:00
Chad Austin
7d34ab2a21 don't implicitly include the repo root in the include path
Summary:
From andrewjcg:

> Basically, rocksdb headers exist at both e.g. rocksdb/src/include/rocksdb/flush_block_policy.h and rocksdb//flush_block_policy.h.  The latter are never meant to be used, but Apple platforms by default add the repo root to the include dir before anything else.  This means depending on file-relative inclusion from within rocksdb, we end up getting different versions of the header, which busts through #pragma once causing multiple definition errors.

This is split out from D15125826.

Reviewed By: andrewjcg

Differential Revision: D16353321

fbshipit-source-id: 095e3b7fb4f56b0be42d18fe4009336b65c8eb5e
2019-07-24 17:44:33 -07:00
Wez Furlong
f5d9a06dc9 eden: add thrift calls for adding/removing bind mounts
Summary:
These allow the cli to setup and tear down mounts and
have the eden server keep track of them.

Reviewed By: strager

Differential Revision: D15707318

fbshipit-source-id: abdb8eaa28c8c67c8211a8af1647efe3a083e998
2019-06-25 18:42:37 -07:00
Puneet Kaushik
452fbb6f58 Add --edenDir and other command line arguments support to Edenfs on Windows
Summary: We need --edenDir support to run muliple instance of Edenfs, which is required to run the integration tests.

Reviewed By: simpkins

Differential Revision: D15951483

fbshipit-source-id: a516159cdeb5f046f795fc28399a2af5fe8a9f95
2019-06-25 14:16:11 -07:00
Adam Simpkins
73509a87f8 restore the $USER environment variable when dropping privileges
Summary:
If edenfs was started using `sudo`, the `$USER` environment variable will be
set to `root` rather than the actual user.  When we drop privileges make sure
we restore the value of `$USER` as well.

The `$USER` variable isn't checked anywhere else in edenfs itself, but it
matters for subprocesses we spawn, like `hg debugedenimporthelper`.

I also changed the code to clear the `SUDO_*` variables as well, mostly
just for good measure.

Reviewed By: kulshrax

Differential Revision: D15929539

fbshipit-source-id: e022c7ae762e2a5e86d0227058bb476aff17cf55
2019-06-20 21:01:36 -07:00
Adam Simpkins
4bc8682391 update license headers in CMake files
Summary:
Update the copyright & license headers in CMake files to reflect the
relicensing to GPLv2

Reviewed By: wez

Differential Revision: D15487079

fbshipit-source-id: 715e559464c19a0070d6e55a095b3fc7d61ad2f8
2019-06-19 17:02:46 -07:00
Adam Simpkins
aa5e6c7295 update license headers in C++ files
Summary:
Update the copyright & license headers in C++ files to reflect the
relicensing to GPLv2

Reviewed By: wez

Differential Revision: D15487078

fbshipit-source-id: 19f24c933a64ecad0d3a692d0f8d2a38b4194b1d
2019-06-19 17:02:45 -07:00
Jon Maltiel Swenson
938eb990a6 Remove defaulted move constructors/assignment operators that are implicitly deleted
Summary:
Some newer versions of `clang` (such as Apple's version 11) will warn/error out if a constructor or assignment operator
marked `default` is implicitly deleted (e.g., if the object contains a non-moveable/non-copyable member). This diff
removes all such defaulted constructors/assignment operators, which I ran into while building `edenfs` on my Macbook Pro.

Reviewed By: chadaustin, strager

Differential Revision: D15901794

fbshipit-source-id: 794ed8377693a6735bb567635dc919bc678751a4
2019-06-19 14:27:25 -07:00
Yedidya Feldblum
c1f6f20511 Replace inclusions of folly/futures/helpers.h with folly/futures/Future.h
Summary: Replace inclusions of `folly/futures/helpers.h` with `folly/futures/Future.h` to avoid the cyclic include trap.

Differential Revision: D15600549

fbshipit-source-id: 19950be24a7437fb1fbec293e24058adf17343ca
2019-06-06 11:09:10 -07:00
Wez Furlong
3528f40c14 eden: report the paths in failed bind mount attempts
Summary: This helps with troubleshooting problems with bind mounts

Reviewed By: simpkins

Differential Revision: D15632617

fbshipit-source-id: 5fd6aca42b48c7797a91012e5857051a415ab725
2019-06-05 07:26:13 -07:00
Matt Glazar
761d5d2c8e Separate FUSE and Hg counter structs
Summary:
Some threads update FUSE counters, some threads update HgBackingStore counters, and some threads update HgImporter counters. No thread updates all of FUSE counters, HgBackingStore counters, and HgImporter counters. FUSE threads and HgImporter threads allocate the full set of counters (EdenThreadStats), even though many of the allocated counters can never be used during the thread's lifetime.

Reduce redundant allocation (and overhead during counter aggregation): split EdenThreadStats into three classes (FuseThreadStats, HgBackingStoreThreadStats, and HgImporterThreadStats) so different threads can allocate different sets of counters.

This diff should not change observable behavior.

Reviewed By: simpkins

Differential Revision: D14822273

fbshipit-source-id: cfd238187d20a0b8d3959673401ecad894e2095b
2019-05-29 18:11:56 -07:00
Adam Simpkins
67cd4bbf45 add a TestServer class
Summary:
Add a helper class for creating an EdenServer in a unit test.

Most of our existing unit tests only create EdenMount objects, without a full
EdenServer.  This new class will make it easier to write tests for
functionality that does require a full EdenServer object.

Reviewed By: chadaustin

Differential Revision: D15492166

fbshipit-source-id: f8b1ce3b78a1160a5d55d305e6bf4b5305cca509
2019-05-28 21:39:39 -07:00
Chad Austin
42dcf78aad clang-format
Summary: Small parts of our code have diverged from our clang-format rules.

Reviewed By: strager

Differential Revision: D15380260

fbshipit-source-id: f668ac22d6c0c5f2468549f2a94dd1c9bb22ce3d
2019-05-17 10:19:02 -07:00
Wez Furlong
b5b76236e0 eden: adjust volume name on macos
Summary:
The Finder shows the volume name instead of the directory
name when it renders the directory containing a mount, and the default
name of `OSXFuse Volume X` makes it hard to distinguish between multiple
mounts in the same folder.

This changes the volume name to be the basename of the mounted path so
that the finder renders things more reasonably.

Reviewed By: chadaustin

Differential Revision: D14969807

fbshipit-source-id: de9d8469360712828598fed124ed02ee5d6dd44a
2019-04-17 12:54:51 -07:00
Wez Furlong
0f2d07002a eden: allow apple double
Summary:
Unfortunately, some node modules packages include `.DS_Store` files
and since we disable apple double, the kext refuses to allow them to be
unpacked in the repo.  Ideally they wouldn't be in the repo, but we need to
unblock that workflow.

Reviewed By: chadaustin

Differential Revision: D14969808

fbshipit-source-id: f9262294a223fd5cc7929a2b324f2401e1fed083
2019-04-17 12:54:51 -07:00
Lee Howes
9c5c50a389 Replace Future::onError with Future::thenError
Summary:
Replace Future::onError with Future::thenError:
 * to remove ambiguous typing
 * to ensure that the executor is not lost and the returned Future is still bound to an executor

See:
https://fb.workplace.com/groups/fbcode/permalink/2002251863144976/
for details.

Reviewed By: Orvid

Differential Revision: D14975808

fbshipit-source-id: b7790bf0e8965c9ffb3e63901bcdaf108480d4ca
2019-04-17 11:59:13 -07:00
Matt Glazar
1057039562 Encapsulate EdenStats
Summary:
EdenStats is currently an alias for ThreadLocal<EdenThreadStats>. I want to split EdenThreadStats into two structs which are allocated independently, but EdenStats's interface makes this impossible.

Refactor EdenStats from an alias to a class and encapsulate the underlying ThreadLocal<EdenThreadStats> member.

This diff should not change behavior.

Reviewed By: simpkins

Differential Revision: D14822272

fbshipit-source-id: 691f4731aa22ecbdcd3535ee0bb0b99c816ffc3d
2019-04-14 20:45:16 -07:00
Matt Glazar
11195c2b0b Rename ThreadLocalEdenStats to EdenStats
Summary:
I'm confused by the naming of EdenThreadStats and ThreadLocalEdenStats. For example, when I see "ThreadLocalEdenStats", I think that such objects can only be accessed by one thread. That's definitely not what "ThreadLocalEdenStats" means!

I think the following naming scheme makes more sense:

* **EdenThreadStats**: Statistics which are updated by one thread. Currently called EdenThreadStats.
* **EdenStats**: Statistics for all threads. Provides access to EdenThreadStats. Currently called ThreadLocalEdenStats.

Implement my preferred scheme: rename ThreadLocalEdenStats to EdenStats.

This diff should not change behavior.

Note: Prior to D14822274, EdenThreadStats was called EdenStats.

Reviewed By: simpkins

Differential Revision: D14822271

fbshipit-source-id: bd20179b1010588e3fc16dc9ed0657d458606f16
2019-04-14 20:45:16 -07:00
Matt Glazar
8bfb4fe9cb Rename EdenStats to EdenThreadStats
Summary:
I'm confused by the naming of EdenStats and ThreadLocalEdenStats. For example, when I see "ThreadLocalEdenStats", I think that such objects can only be accessed by one thread. That's definitely not what "ThreadLocalEdenStats" means!

I think the following naming scheme makes more sense:

* **EdenThreadStats**: Statistics which are updated by one thread. Currently called EdenStats.
* **EdenStats**: Statistics for all threads. Provides access to EdenThreadStats. Currently called ThreadLocalEdenStats.

Implement half of my preferred scheme: rename EdenStats to EdenThreadStats. (Make this diff easier to read by renaming ThreadLocalEdenStats to EdenStats in a separate diff.) In effect, implement the following naming scheme:

* **EdenThreadStats**: Statistics which are updated by one thread. (was EdenStats)
* **ThreadLocalEdenStats**: Statistics for all threads. Provides access to EdenThreadStats. (no change)

This diff should not change behavior.

Reviewed By: simpkins

Differential Revision: D14822274

fbshipit-source-id: 236cd9878cd249a06d14e124050ee01329667a18
2019-04-14 20:45:16 -07:00
Matt Glazar
d9e4eabc9d Move EdenStats into eden/fs/tracing/
Summary:
I want to use EdenStats in eden/fs/store/. EdenStats currently lives in eden/fs/fuse/, and making eden/fs/store/ depend upon eden/fs/fuse/ is confusing. (It's also confusing that some code in eden/fs/fuse/ is used on Windows.)

Reorganize the code: move EdenStats into eden/fs/tracing/.

This diff should not change behavior.

Reviewed By: chadaustin

Differential Revision: D14677337

fbshipit-source-id: af26d214bcc3a9919920fbd4e59e6098fe4e3834
2019-04-01 17:41:57 -07:00
Chad Austin
c804232e52 opt into readdir caching if the kernel supports it but not FUSE_NO_OPENDIR_SUPPORT
Summary:
When Eden runs on kernel 4.20 (has readdir caching but not
FUSE_NO_OPENDIR_SUPPORT) indicate to the kernel that Eden wants
readdir results to be cached.

Reviewed By: wez

Differential Revision: D13893922

fbshipit-source-id: 3092adb16dabc4273bdba1e6e9f15e9e3bd7bb87
2019-03-22 15:57:33 -07:00
Chad Austin
cc1c841004 stop handling opendir and releasedir on kernels with FUSE_NO_OPENDIR_SUPPORT
Summary:
Eden requires no state in its directory handles, so tell the kernel it
doesn't need to send opendir() and releasedir() requests, provided it
has FUSE_NO_OPENDIR_SUPPORT.

Reviewed By: strager

Differential Revision: D13594734

fbshipit-source-id: ebd4b69f4efcd1428a69024c4bdffb1ae455fa40
2019-03-22 15:57:33 -07:00
Chad Austin
1a1dbccb1d add osxfuse kernel header
Summary:
Eden needs the osxfuse version of the FUSE protocol on mac. The open
source cmake build pulls in osxfuse with getdeps.py, but that doesn't
work in the Buck build, and all we need is this one (old) copy of the
Linux kernel headers.

Reviewed By: strager

Differential Revision: D14506747

fbshipit-source-id: 028ddbaf80be9cad412462f3338c30b8f2a70087
2019-03-19 10:26:24 -07:00
Zeyi Fan
2b086dfe3e fix oss macOS build
Summary: This diff fixes Eden build on macOS.

Reviewed By: chadaustin

Differential Revision: D14507115

fbshipit-source-id: fdf460ebadc2e69b1cf3fc40a6fd3e104dbc2833
2019-03-18 14:12:57 -07:00
Adam Simpkins
0e9d62783d tweak error log messages for unhandled fuse opcodes.
Summary:
Update the FuseChannel code to explicitly handle `FUSE_LSEEK` and `FUSE_POLL`
to avoid emitting error log messages about these calls not being implemented.
We intentionally do not implement these operations at the moment.

Also drop the log about other unknown opcodes from an error log to a warning
message.

Reviewed By: chadaustin

Differential Revision: D14453246

fbshipit-source-id: 2605cf7e5c160cda92460a80187438ac3549ade5
2019-03-14 13:07:09 -07:00
Adam Simpkins
d529f75e9e tweak the log levels of some FUSE log messages
Summary:
I want to change some of the StartupLogger behavior to automatically show all
INFO and higher level log messages to the user during start-up.  These two
messages from the FUSE code are currently logged at the INFO level, but don't
really seem important enough to show to the user.

This drops their level to DBG1.  These messages will therefore still be shown
in the normal `edenfs.log` file, since it includes everything from DBG2 and
up.

Reviewed By: chadaustin

Differential Revision: D14453247

fbshipit-source-id: 5d79766f87e658b807029d82ac035cb94ae68832
2019-03-14 13:07:09 -07:00
Matt Glazar
201fef1c2e Create exception type for FUSE unmount-during-init
Summary:
FuseChannel::initialize throws runtime_error when the FUSE connection is interrupted. I want EdenMount::startFuse to handle unexpected-unmount errors from FuseChannel::initialize, but catching runtime_error would catch unrelated errors too.

When the FUSE connection is interrupted during initialization, make FuseChannel throw FuseDeviceUnmountedDuringInitialization instead of runtime_error.

Reviewed By: chadaustin

Differential Revision: D14077848

fbshipit-source-id: ed7b7d370a83ed1a9c36a443d8bb06ba940dc032
2019-03-11 20:48:27 -07:00
Yedidya Feldblum
92a8e3f59f Stop checking EventBase::runInEventBaseThread result
Summary:
[Folly] Stop checking `EventBase::runInEventBaseThread` result, as the function will soon be changed not to return any result.

It returned `false` when failing to enqueue a task. But it cannot really fail anyway besides allocation failure, unless in the `EventBase` destructor and while draining and the `AlwaysEnqueue` variant is called.

Reviewed By: andriigrynenko

Differential Revision: D14254969

fbshipit-source-id: a6a9199cbafa18b61488a240e4318ce946953f51
2019-02-28 02:36:15 -08:00
Chad Austin
015c45872b split privhelper and privhelper_base
Summary:
We're not that far from building privhelper on mode/mac but it does
require figuring out how to depend on osxfuse from the Buck build, so
bypass that by breaking the inodes target's dependency on privhelper's
<fuse_ioctl.h> include.

Reviewed By: strager

Differential Revision: D14218709

fbshipit-source-id: edbb2a21df06d6f2a4f860ef13718ad05d445e98
2019-02-27 19:06:00 -08:00
Chad Austin
b8dc8a80c3 only include selinux on linux
Summary: We should only attempt to include selinux on Linux, not macOS.

Reviewed By: strager

Differential Revision: D14181109

fbshipit-source-id: be47b7bdadc3409577fa114559e905214848ebd8
2019-02-22 16:14:48 -08:00
Michael Liu
b626f922ce Apply modernize-use-override (2nd iteration)
Summary:
Use C++11’s override and remove virtual where applicable.
Change are automatically generated.

Reviewed By: simpkins

Differential Revision: D14087291

fbshipit-source-id: 80e6a393c5ed8ea1656855da3832bcee10635004
2019-02-14 17:29:27 -08:00
Chad Austin
1c84f3f115 split InodeNumber into its own file
Summary:
It's common for code to use InodeNumber without needing to include the
main FUSE headers or vice versa. Split them into two separate headers.

Reviewed By: strager

Differential Revision: D13979868

fbshipit-source-id: c5eeb6a3697bb538729a403434dc4f0f7408cda0
2019-02-08 16:21:35 -08:00
Matt Glazar
25327947e0 Delete dead definition of InodeNumber
Summary: FuseTypes.h defines the InodeNumber struct, but also has a comment which implies that InodeNumber is an alias of uint64_t. Delete this misleading commented-out definition of InodeNumber.

Reviewed By: chadaustin

Differential Revision: D13892907

fbshipit-source-id: 8e9710db29d8e0d708ee1785956f6953a0b2a49f
2019-01-31 12:59:48 -08:00
Chad Austin
c97631eba4 log sendInvalidateInode and sendInvalidateEntry calls and ENOENT errors
Summary:
When debugging Eden's behavior under a kernel that caches readdir
calls, I observed that directories were not returning the correct
entries after a checkout. The issue is that Eden calls INVAL_ENTRY
when new entries are added, but INVAL_ENTRY is a no-op and returns
ENOENT if the kernel doesn't know about that entry.

Reviewed By: strager

Differential Revision: D13864281

fbshipit-source-id: 56b3a67d0bc6f8ed1733acaf5e889414b753cbc7
2019-01-30 23:00:38 -08:00
Matt Glazar
0b151df7b3 Delete unused function declaration
Summary: PrivHelperServer::initLogging is declared in the .h file, but it's never defined. Remove the useless declaration.

Reviewed By: simpkins

Differential Revision: D13814416

fbshipit-source-id: 25cb47442a19947da08d13d9bed9b4631a1c9739
2019-01-28 11:32:56 -08:00
Jon Maltiel Swenson
79b7db4d91 Make EventBase destruction callbacks safely cancellable
Summary: Currently, `runOnDestruction` aims to be thread-safe; new callbacks are added to the `onDestructionCallbacks_` list while the associated mutex is held. However, the caller may own the `LoopCallback` and wish to destroy/cancel it before the `EventBase` destructor runs, and this callback cancellation is not thread-safe, since unlinking does not happen under the lock protecting `onDestructionCallbacks_`. The primary motivation of this diff is to make on-destruction callback cancellation thread-safe; in particular, it is safe to cancel an on-destruction callback concurrently with `~EventBase()`.

Reviewed By: spalamarchuk

Differential Revision: D13440552

fbshipit-source-id: 65cee1e361d37647920baaad4490dd26b791315d
2019-01-24 15:57:39 -08:00
Chad Austin
4f532889d1 reenable ProcessNameCache
Summary:
Now that the deadlock in ProcessNameCache has been fixed, bring it
back.

Reviewed By: strager

Differential Revision: D13742965

fbshipit-source-id: f407105e06b9954766bdb48ef1303e2003c07284
2019-01-24 15:45:29 -08:00
Wez Furlong
2ba72dc18e eden: have priv helper load the fuse kext if needed
Summary:
This is something that is not needed on linux because
the kernel module is typically already loaded (at least on the systems
on which we run).

On macos, since fuse is not part of the kernel, libfuse has some code
that loads it when needed.

This diff performs the equivalent actions for eden.

Reviewed By: simpkins

Differential Revision: D13721489

fbshipit-source-id: 627bc90681141d0e7da3d5b5e06756a36839958c
2019-01-17 18:52:53 -08:00
Wez Furlong
4e596a944b eden: implement mount/unmount for osxfuse
Summary:
Note that the concept of bind mounts doesn't exist on macos, so that
portion of the server just throws.

Reviewed By: simpkins

Differential Revision: D13480147

fbshipit-source-id: 92225188c0af42574d090004490f3926d393747b
2019-01-17 18:52:53 -08:00
Wez Furlong
1fac9783e3 eden: remove fuse request interrupt code, track requests by internal id
Summary:
This is really a continuation of D13479516; the issue is that
the osxfuse kernel module is very eager to recycle `unique` request
id values, recycling them before our code has had a chance to update
internal state.

This diff re-keys the requests map so that we generate our own sequence
of identifiers to use as the key rather than the fuse protocol `unique`
value.

Because we cannot reliably track by `unique` value we also cannot
reliably implement interrupt support.  We've never really tested
interrupt support, and it relies on functionality in folly futures
that hasn't really been tested or proven either, so I've removed
that functionality as part of this diff.

That allows simplifying some code in RequestData and FuseChannel;
we're now able to simply tack an `.ensure` on the end of the
future chain to ensure that we remove the entry from the map
once the future is resolved, successfully or otherwise.

Reviewed By: chadaustin

Differential Revision: D13679964

fbshipit-source-id: c1081a868c4061de2a725589ec1614959a8e9316
2019-01-16 14:35:33 -08:00
Wez Furlong
8dcc59c36b eden: improve debug logging for errors and requests
Summary: This just makes the debug a little easier to follow.

Reviewed By: chadaustin

Differential Revision: D13680020

fbshipit-source-id: e4045822e56ba42a831ccb0ceaa9baaba5b79a10
2019-01-15 17:29:24 -08:00
Wez Furlong
54d86f2574 eden: fixup attributes with osxfuse
Summary:
the osxfuse implementation includes some additional
fields in the `fuse_attr` struct.  One of those fields is
`flags`.  We were not initializing this field when converting
the stat data to fuse attributes which resulted in it holding
"random" data.  In debug builds this seemed to usually end up
zeroed out but in release builds it was usually a bit pattern
that caused the kernel to respond with EPERM when attempting
to access the files.   I didn't capture exactly what that
bit pattern was, just that initializing the struct to zeroes
reliably fixed up the EPERM issues in the Release build.

Reviewed By: chadaustin

Differential Revision: D13680004

fbshipit-source-id: 6b2ce6c10ef8f7db4a8a50bd3f2ddcfdddc3bb45
2019-01-15 17:29:24 -08:00
Wez Furlong
13c2d03331 eden: fixup compilation warning with clang
Summary:
Address this error with clang:

```
In file included from /Users/wez/fbsource/fbcode/eden/oss/eden/fs/service/main.cpp:25:
/Users/wez/fbsource/fbcode/eden/oss/eden/fs/fuse/privhelper/PrivHelper.h:22:1: warning: class 'Unit' was previously declared as a struct [-Wmismatched-tags]
class Unit;
^
/Users/wez/fbsource/fbcode/eden/oss/external/install/include/folly/Unit.h:36:8: note: previous use is here
struct Unit {
       ^
/Users/wez/fbsource/fbcode/eden/oss/eden/fs/fuse/privhelper/PrivHelper.h:22:1: note: did you mean struct here?
class Unit;
^~~~~
struct
```

Reviewed By: strager

Differential Revision: D13602383

fbshipit-source-id: 6e69716498680660181ab441c3c007b074ec1d40
2019-01-14 13:52:36 -08:00
Chad Austin
09a27f492a debug logging (once) when ENODEV is received from the fuse device
Summary:
To determine the precise behavior of unmounting an Eden mount with
MNT_FORCE, I needed to see whether ENODEV was being returned from the
FUSE socket.

This helps us clarify the documentation in libfuse
https://github.com/libfuse/libfuse/issues/333

Reviewed By: strager

Differential Revision: D13630289

fbshipit-source-id: 90e6f0afc927c042a24cd6c82deac644c15ed066
2019-01-11 15:11:14 -08:00
Matt Glazar
e9f5639b1f Make ProcessNameCache optional for ProcessAccessLog
Summary:
To mitigate a deadlock, I want to make ProcessAccessLog not access /proc/. Allow this by making ProcessNameCache optional.

This diff should not change behavior.

Reviewed By: simpkins

Differential Revision: D13540948

fbshipit-source-id: 4c5d68c972c04122de1d2414084debfec078dd4c
2018-12-21 15:43:51 -08:00
Chad Austin
26a244f8c8 enable FUSE_PARALLEL_DIROPS
Summary:
Eden can handle parallel readdir and lookup so don't require the
kernel to acquire a mutex just to serialize the requests.

5c672ab3f0

Reviewed By: strager

Differential Revision: D13386133

fbshipit-source-id: aa935af941ff2901b07b63751c97052c295f7076
2018-12-18 13:03:45 -08:00
Wez Furlong
8190b08254 eden: fixup some fuse version differences for osxfuse
Summary:
The fuse opcodes are defined as an enum so we have to use
the relatively coarse and indirect apple vs linux preprocessor
checks in the maps for the opcode names.

The osxfuse implementation branched off from the 7.19 fuse
implementation, so add a light dusting of some preprocessor
checks around enabling the performance optimization features
we desire on Linux.

We also need to relax the compile time check for the min
fuse version; I've constrained this to be apple specific,
although I suppose it wouldn't hurt to make it more broadly
applicable.

Reviewed By: chadaustin

Differential Revision: D13480145

fbshipit-source-id: 010ac114e22ea942dfcebf1105cb1f01b766f297
2018-12-17 20:16:19 -08:00
Wez Furlong
0774b2c457 eden: pull in osxfuse kernel headers
Summary:
There's nothing nice about this; the full set of kernel headers are
not installed with the binary distribution, and since the kernel distribution
has to be signed to be loaded on osx, there's no benefit to us building it
for ourselves.

This diff adds a nop builder and tweaks the cmake to point into the osxfuse
repo.

The osxfuse repo aggregates a couple of related repos using the git
submodule feature, so trigger that from getdeps.py too.

Reviewed By: strager

Differential Revision: D13480148

fbshipit-source-id: 84e09a86f6a83f83ffd1e3fe113dc7b15b3ea208
2018-12-17 20:16:19 -08:00
Wez Furlong
b69536b6d4 eden: no SOCK_CLOEXEC on darwin
Summary:
handle this best-effort by setting this bit on each fd after
allocating them.

Reviewed By: strager

Differential Revision: D13475712

fbshipit-source-id: 46be80f025b21967f75822f983bc327c5e2d20af
2018-12-17 20:16:18 -08:00
Wez Furlong
60645f897d eden: forward decl of DirList is insufficient for clang on macos
Summary:
Avoid this compilation error:

```
[ 23%] Building CXX object eden/fs/fuse/CMakeFiles/eden_fuse.dir/Dispatcher.cpp.o
In file included from /Users/wez/fbsource/fbcode/eden/oss/eden/fs/fuse/Dispatcher.cpp:10:
In file included from /Users/wez/fbsource/fbcode/eden/oss/eden/fs/fuse/Dispatcher.h:12:
In file included from /Users/wez/fbsource/fbcode/eden/oss/external/install/include/folly/Portability.h:19:
In file included from /Library/Developer/CommandLineTools/usr/include/c++/v1/cstddef:110:
/Library/Developer/CommandLineTools/usr/include/c++/v1/type_traits:3141:38: error: incomplete type 'facebook::eden::DirList' used in type trait expression
    : public integral_constant<bool, __is_constructible(_Tp, _Args...)>
                                     ^
/Users/wez/fbsource/fbcode/eden/oss/external/install/include/folly/futures/Future.h:146:36: note: in instantiation of template class 'std::__1::is_constructible<facebook::eden::DirList>' requested here
      typename std::enable_if<std::is_constructible<T, Args&&...>::value, int>::
                                   ^
/Users/wez/fbsource/fbcode/eden/oss/external/install/include/folly/futures/Future.h:148:12: note: while substituting prior template arguments into non-type template parameter [with Args = <>]
  explicit FutureBase(in_place_t, Args&&... args);
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/wez/fbsource/fbcode/eden/oss/external/install/include/folly/futures/Future.h:115:7: note: while substituting deduced template arguments into function template 'FutureBase' [with Args = <>, $1 = (no value)]
class FutureBase {
      ^
/Users/wez/fbsource/fbcode/eden/oss/eden/fs/fuse/Dispatcher.cpp:175:13: note: in instantiation of template class 'folly::Future<facebook::eden::DirList>' requested here
Dispatcher::readdir(InodeNumber, DirList&&, off_t, uint64_t) {
            ^
/Users/wez/fbsource/fbcode/eden/oss/eden/fs/fuse/Dispatcher.h:35:7: note: forward declaration of 'facebook::eden::DirList'
class DirList;
      ^
1 error generated.
```

Reviewed By: chadaustin

Differential Revision: D13475720

fbshipit-source-id: 2da1692010a82b73cbed71d996993cf5fc13af0e
2018-12-17 15:53:15 -08:00
Wez Furlong
299e22d940 eden: avoid DCHECK in FuseChannel code on macOS
Summary:
It appears as though osxfuse is a bit eager to re-use `header->unique`
values, as this DCHECK to ensure that we erased our request at the end of a
request is frequently triggered.  It's not for 100% of requests, but I did
notice that the `header->unique` values tend to be in the range 1-6.

We don't check for collisions when we emplace requests in the dispatching
logic, and this particular check happens after we've acked the kernel, so it
doesn't seem like a safe thing to check and abort on really, so let's remove
it.

Reviewed By: chadaustin

Differential Revision: D13479516

fbshipit-source-id: a01e6c3e47b78b651f65fc3f5138168c71968030
2018-12-17 15:53:15 -08:00
Wez Furlong
3dbcd058d6 eden: add helper for accessing stat fields as timespecs
Summary:
ported forward from D4209167, add a couple of helpers
to access these fields on mac and linux, centralizing/minimizing ifdefs.

Simplify some of the logic in FileChangeMonitor.

Reviewed By: chadaustin

Differential Revision: D13475717

fbshipit-source-id: d7b39999808bc41a6dc17a87189501cb34e68b30
2018-12-16 18:31:46 -08:00
Wez Furlong
ae390beb6e eden: prctl is linux specific
Summary: only include and use it on linux

Reviewed By: simpkins

Differential Revision: D13475715

fbshipit-source-id: 6b0b9da1b32088e01cbb932f9b3ed62532dfe00f
2018-12-15 13:43:31 -08:00
Wez Furlong
31eb2b1c8c eden: selinux is linux specific, adjust cmake accordingly
Summary: only add the selinux deps if we found selinux

Reviewed By: simpkins

Differential Revision: D13475711

fbshipit-source-id: c3375282b61881317f9a6c4c8e321ce717d1f9ab
2018-12-15 13:43:31 -08:00
Wez Furlong
3b5315ca43 eden: handlemap.thrift was removed, but not from cmake build
Summary: overlooked because there is no CI exercising this today

Reviewed By: simpkins

Differential Revision: D13475721

fbshipit-source-id: 3e8fe280ab73d249da374129b37d32cd7e17f472
2018-12-15 13:43:31 -08:00
Chad Austin
008497c69a remove SerializedFileHandleMap
Summary: SerializedFileHandleMap is dead code now.

Reviewed By: strager

Differential Revision: D13381629

fbshipit-source-id: ba872aaf8335d2be68d6af0465bd04e4ca59d578
2018-12-13 12:29:13 -08:00
Chad Austin
1e83ec3df0 remove FileHandleMap
Summary:
Eden no longer tracks any state in file handles, and has no plans to in the future.
Therefore, remove all related code.

Reviewed By: strager

Differential Revision: D13354307

fbshipit-source-id: 341d081f64c6c8fb2b4b1b5a5ff42f2cc7d38039
2018-12-13 12:29:13 -08:00
Chad Austin
6e0ce0ace0 stop handling FUSE_OPEN
Summary:
Now that all file access in Eden is stateless, we no longer need to handle open() or release().
If the kernel advertises FUSE_NO_OPEN_SUPPORT, return ENOSYS from open().

Reviewed By: simpkins

Differential Revision: D13325759

fbshipit-source-id: 38486848f27ffeb005f74407888e94d891496f98
2018-12-12 21:49:07 -08:00
Adam Simpkins
4ddda70198 unbreak open()
Summary:
D13325746 changed `EdenDispatcher::open()` to no longer return a file handle.
However the code in `FuseChannel::fuseOpen()` throws an exception if the
dispatcher does not create a file handle.  This breaks most file operations in
Eden.

The Dispatcher check is removed in D13354307, but that hasn't landed yet.
That change probably just needed to be part of D13325746.

Reviewed By: chadaustin

Differential Revision: D13445570

fbshipit-source-id: 70d639142057740766bcbe02a0df50b14f7c9937
2018-12-12 20:32:32 -08:00
Chad Austin
5222e339e6 remove EdenFileHandle and FileInode::open
Summary: Title says it all.

Reviewed By: strager

Differential Revision: D13325746

fbshipit-source-id: 22f1b12ba0bf47eba62c2312e5069c45b1c28ef3
2018-12-12 17:10:29 -08:00
Chad Austin
a978af2c62 stop looking up file handles in FuseChannel
Summary:
Previously, a file handle must have been held for the entirety of a write operation. That is no
longer true. Stop looking up file handles on write.

Reviewed By: strager

Differential Revision: D13325662

fbshipit-source-id: 9ae31b467d17d633c388917d18098e6e5a620b89
2018-12-12 17:10:29 -08:00
Chad Austin
ef141a6585 remove DirHandle
Summary: DirHandle no longer does anything. Remove it.

Reviewed By: strager

Differential Revision: D13288298

fbshipit-source-id: 3edebbcdf60982608ddb87c1ff82ebff1c3d2067
2018-12-05 01:34:53 -08:00
Chad Austin
1b9fc49bfc simplify readdir implementation
Summary:
Write tests for readdir's semantics (we really do want to return . and
..) and simplify the logic. It's still quadratic in large directories,
but there aren't any allocations anymore.

Reviewed By: strager

Differential Revision: D13287764

fbshipit-source-id: 5e0d4b86eb16dbd7a16cdeb324e4b43363512e25
2018-12-05 01:34:53 -08:00
Chad Austin
c55edc9036 route readdir straight to TreeInode
Summary:
Send readdir requests to TreeInode. This may not sound like a good
idea: the FUSE documentation suggests that stateful directory handles
are required to implement correct readdir semantics under concurrent
deletes and renames. However, the 63-bit offset value is treated as a
cookie that is passed from one readdir call into the next, and 63 bits
should be sufficient to implement readdir concurrent with
rename/unlink. So move readdir's implementation into TreeInode in
preparation for the complete removal of TreeInodeDirHandle.

Reviewed By: strager

Differential Revision: D13287664

fbshipit-source-id: c0d615675edd9b83353534468a69b89068bba923
2018-12-04 16:37:41 -08:00
Chad Austin
c6617a0649 don't route fsyncdir through DirHandle
Summary:
Send fsyncdir straight through the inode rather than going through
DirHandle. This is the better design anyway, since the DirHandle does
not receive directory-mutating requests like mkdir.

Reviewed By: strager

Differential Revision: D13287610

fbshipit-source-id: 154fa32a3877c89a204a2d10b4e2b637410d9486
2018-12-03 17:43:34 -08:00
Chad Austin
84c5fe913d be explicit that we don't plan to use ATOMIC_O_TRUNC
Summary:
FUSE_NO_OPEN_SUPPORT is better than ATOMIC_O_TRUNC for Eden's use
case. Remove the code that pretended we might support ATOMIC_O_TRUNC
again someday.

(Note: this ignores all push blocking failures!)

Reviewed By: strager

Differential Revision: D13163382

fbshipit-source-id: 948d701571a8d2977da3d2532fdc9538c5011636
2018-11-29 11:22:58 -08:00
Chad Austin
a6b4e0f2ff add CacheHint parameters to FileInode reads
Summary:
The new blob cache wants to know, given a request, whether the blob is
expected to be needed or not. The answer, in general, is yes if the
request came from Thrift and no if it came from FUSE, because the kernel
will cache the result of the request in its own page and dentry caches.
Propagate this information through FileInode.

Reviewed By: strager

Differential Revision: D12813838

fbshipit-source-id: 7a359686149cd4daff41630c94085b680c448c4f
2018-11-22 00:45:25 -08:00
Dan Schatzberg
8fe62ce81b Add command to chown a mount
Summary:
Sandcastle has several cases where we chown the entire
repository which performs terribly on Eden. As a workaround we have a
command to do this in eden without loading all the files.

Reviewed By: chadaustin

Differential Revision: D12857956

fbshipit-source-id: 36cebcc710fbcf4e1eb265df901513cf50a227b9
2018-11-07 08:58:31 -08:00
Dan Schatzberg
4efb8d24a5 Use FUSE_CACHE_SYMLINKS option
Summary: This option was added upstream and to our internal 4.16 kernel series

Reviewed By: chadaustin

Differential Revision: D12823476

fbshipit-source-id: 54d241b77eff92f4083c0f1d4c98b47a6a098e3f
2018-10-29 11:17:14 -07:00
Chad Austin
8cf1e44823 folly::Optional -> std::optional
Summary: Eden's on C++17 so fully cross the rubicon!

Reviewed By: strager

Differential Revision: D10498098

fbshipit-source-id: 33a885614d627399645b70a54092c1badd795fbe
2018-10-23 18:51:59 -07:00
Adam Simpkins
dacc8787f3 convert some deprecated Future::then() calls
Summary:
Convert deprecated `folly::Future::then()` calls to `thenTry()` or
`thenValue()` as appropriate.

Reviewed By: chadaustin

Differential Revision: D10503906

fbshipit-source-id: abc0f6f588ad7edd0dd2576544875f4ad0263b83
2018-10-23 13:42:12 -07:00
Adam Simpkins
2904016208 cmake: fix the build when SELinux is available
Summary:
D9029272 (github commit fae2056037) accidentally broke the CMake-based
build of Eden when SELinux is available: it defined `PRIVHELPER_LIBS` but
later used `PRIVHELPER_LIBRARIES` (LIBS vs LIBRARIES).

This fixes that error by simply removing this intermediate variable.  We
can just use SELINUX_INCLUDE_DIR and SELINUX_LIBRARIES directly, as they
will simply be empty if SELinux is not available.

Reviewed By: quark-zju

Differential Revision: D10503905

fbshipit-source-id: 1c12bb1cad0351e4e0a77d0c7e8a83086209efee
2018-10-23 13:42:12 -07:00
Puneet Kaushik
1e94122c0b Win: Porting eden/service to Windows with fb-thrift
Reviewed By: strager

Differential Revision: D10225935

fbshipit-source-id: 5c571229e243b8fcabb9db8a6e15e1c66423edaf
2018-10-22 20:27:26 -07:00
Chad Austin
be065e5997 allow reading xattrs from files after getxattr is called on a directory
Summary:
Eden supports reading the SHA-1 of a file via getxattr. Unfortunately,
it returned ENOSYS if you called getxattr on a directory inode. This
caused FUSE to fail all future getxattr requests with EOPNOTSUPP.

In addition to fixing that, this diff makes our xattr handling a
little more consistent across inodes:

- setxattr() always fails with ENOSYS
- removexattr() always fails with ENOSYS
- listxattr() is always handled by the corresponding inode class
- getxattr() is always handled by the corresponding inode class

Differential Revision: D10437723

fbshipit-source-id: a1ea1e92d3412abb15e91057becea3160a17f1e2
2018-10-22 20:27:26 -07:00
Chad Austin
fffabfe6a0 support running on kernels that require smaller fuse_init_out responses
Summary:
This diff dynamically detects whether eden is running on an older
kernel and sends back a smaller fuse_init_out response.

Reviewed By: simpkins

Differential Revision: D10282016

fbshipit-source-id: ce9701e8f39defd4b90b15fc941ad2e243ac61e9
2018-10-10 13:09:19 -07:00
Chad Austin
cb4674d514 move write from EdenFileHandle to FileInode
Summary: Always send write requests straight to the inode rather than going through FileHandle.

Reviewed By: wez

Differential Revision: D10220619

fbshipit-source-id: 9ce328583cf0fa9d7d8850d92d9e15ddc382d6a3
2018-10-08 15:11:55 -07:00
Chad Austin
462522898d move read from EdenFileHandle to FileInode
Summary:
Always send read requests straight to the inode rather than going
through the FileHandle.

Reviewed By: wez

Differential Revision: D10220604

fbshipit-source-id: 6aa5d20f3ce09696a29bd5c1cb95d0b987ab213c
2018-10-08 15:11:55 -07:00
Chad Austin
369d85108f remove getattr and setattr from FileHandleBase
Summary:
Always send setattr and getattr straight to the inode rather than
going through the FileHandle.

Reviewed By: wez

Differential Revision: D10187876

fbshipit-source-id: 4c3aaa977cd568d5f9cc4b28583e164119c07c1b
2018-10-08 13:17:03 -07:00
Chad Austin
b38171cb08 move fsync and flush from EdenFileHandle to FileInode
Summary: Clip more logic from EdenFileHandle.

Reviewed By: wez

Differential Revision: D10187239

fbshipit-source-id: f11090e23bd1d6e61414e4d9455509e0dca305f2
2018-10-08 11:13:38 -07:00
Chad Austin
59ad0c2ac8 store the file handle -> InodeNumber map in the FileHandleMap
Summary: Eliminate the need to look up an InodeNumber from a FileHandle. Instead, simply preserve the mapping when it's created.

Reviewed By: wez

Differential Revision: D10187120

fbshipit-source-id: dc47f7776294871ff2398f33c31bd85d240ead50
2018-10-08 11:13:38 -07:00
Chad Austin
876b4f985a collapse three FileHandle bits because they're constant in practice
Summary:
In order to implement FUSE_NO_OPEN_SUPPORT, we must eliminate
FileHandle and FileHandleBase. They didn't add any value anyway. Start
clipping.

Reviewed By: wez

Differential Revision: D10187103

fbshipit-source-id: 81e226f9c12486e0bbbde99b798b169fa31740c2
2018-10-08 11:13:38 -07:00
Chad Austin
dbb793becf Don't crash when handling FUSE requests from own edenfs
Summary:
Aborting the process with self-access is overlay aggressive. We
already respond with EIO, and the calling syscall should handle the
error. Continue logging at CRITICAL.

Reviewed By: wez

Differential Revision: D10134987

fbshipit-source-id: a4c4eed5e5de20698e95f442b3e063a09db311e6
2018-10-02 10:07:38 -07:00
Chad Austin
88b08e5b00 clang-format
Summary: We've diverged in a few places from clang-format, so run it across the entirety of Eden.

Reviewed By: wez

Differential Revision: D10137785

fbshipit-source-id: 9603c2eeddc7472c33041ae60e3e280065095eb7
2018-10-02 10:07:38 -07:00
Chad Austin
f23902ae6f To avoid self-access deadlocks, at least respond to FUSE before aborting
Summary:
If an edenfs thread accesses an Eden mount, it trips SIGABRT from the
DFATAL, but the SIGABRT can't end the process because one thread is
stuck in uninterruptible sleep. So respond to the FUSE request before
aborting the process.

Reviewed By: strager

Differential Revision: D10118625

fbshipit-source-id: b84063e0b186d6a464531284b70c25b0b6a710ce
2018-10-01 14:37:23 -07:00
Wez Furlong
1f9d381451 increase information in some fuse debug statements
Summary: I wanted to see more details from the fuse requests, so log them.

Reviewed By: strager

Differential Revision: D9944649

fbshipit-source-id: 143703528fa029ed51e6cb42a5f6d8b3b0230ca3
2018-09-20 12:54:24 -07:00
Yedidya Feldblum
e9e59f8707 Cut assorted dead includes of folly/MoveWrapper.h
Summary: Cut assorted dead `#include`s of `folly/MoveWrapper.h`.

Reviewed By: aary

Differential Revision: D9934365

fbshipit-source-id: f44aa0dd0d8d482e2dc125983929ea8e7fdf974e
2018-09-20 01:54:26 -07:00
Chad Austin
e750ab68fe expose FUSE accesses over Thrift
Summary:
Add a Thrift API for reading the pid access logs from each
EdenMount/FuseChannel. Used in a future diff.

Reviewed By: strager

Differential Revision: D9477867

fbshipit-source-id: 0897a915ca654bca952aecc123ea40105830a75b
2018-09-10 13:52:51 -07:00
Chad Austin
e88177a2f3 count FUSE accesses by process ID
Summary: Begin tracking pids passed into FUSE in the ProcessAccessLog.

Reviewed By: strager

Differential Revision: D9595795

fbshipit-source-id: 02e5fefebcd0de860274409ba6258f14fa050b55
2018-09-10 13:52:51 -07:00
Chad Austin
0cc3121417 Minor FuseChannel optimizations
Summary: While looking at FuseChannel I noticed an unnecessary Future.

Reviewed By: strager

Differential Revision: D9595672

fbshipit-source-id: 5c84822c4f2c4c3c78b88456e44728e463d5a1e8
2018-09-05 15:06:58 -07:00
Dan Schatzberg
e80a31de94 Have tryRlockCheckBeforeUpdate pass a LockedPtr
Summary:
By passing a locked ptr instead of a reference directly to
the locked object, the check and update methods can drop the lock
early

Reviewed By: chadaustin

Differential Revision: D9635507

fbshipit-source-id: a881043cfd2c28f6f53eb12e1494fcbc5f7f8e08
2018-09-05 07:53:57 -07:00
Wez Furlong
8cc1170bf7 tolerate ECHILD when waiting on privhelper process
Summary:
Since the privhelper process is often not our direct child
we both cannot and do not need to wait for it, so treat ECHILD as
a successful exit status to prevent bubbling up an exception that
blocks graceful restart.

Reviewed By: simpkins

Differential Revision: D9581473

fbshipit-source-id: 6d53bbb6ee2043de76df9c6870028a1c15571dac
2018-08-30 17:28:10 -07:00
Lee Howes
c0a837ec7d Future<T>::then Future<T>::then(not-try-task) -> Future<T>::thenValue(task).
Summary:
Overall plan to modify Future<T>::then to be r-value qualified and use Future<T>::thenTry or Future<T>::thenValue.

The goal is to disambiguate folly::Future and to improve type and lifetime safety of Future and its methods.

Codemod:
 * future<T>.then(callable with operator()(not-a-try)) to future<T>.thenValue(callable with operator()(not-a-try)).
 * future<T>.then(callable with operator()()) to future<T>.thenValue(callable with operator()(auto&&)).
 * future<T>.then(callable with operator()(auto)) to future<T>.thenValue(callable with operator()(auto)).

Reviewed By: yfeldblum

Differential Revision: D9512177

fbshipit-source-id: daa3581611dcd9f32d9314bae1c5fa0f966613f3
2018-08-26 22:52:34 -07:00
Marshall Cline
16c7a8bccc Make RequestData::catchErrors() be a template over T that takes Future<T>&&
Summary:
Goal: change catchErrors() so its param is an rvalue-ref (`template <typename T> ... catchErrors(Future<T>&& fut)`) instead of a forwarding ref (and, if necessary, adjust callsites that passed an lvalue-ref).

Rationale: this is part of T33085035 which is migrating Future::onError() to be rvalue-only. D9505757 codemodded `RequestData::catchErrors()` from `fut.onError()` to `std::move(fut).onError()`. However `fut` was a forwarding-ref so it can actually hold an lvalue-ref.

Timeline/urgency: this diff is a prereq to removing the lvalue-qual overload of Future::onError() (T33085035). (Technically the only part of this that is a prereq to T33085035 is to adjust callers of catchErrors() that pass lvalue Futures, but changing the catchErrors() signature is righteous since it will force a more sensible error-message on any future callers which erroneously pass an lvalue.)

Note: D9505757 landed since it didn't break anything, even though it used `std::move()` on a forwarding-ref. (The lvalue-qual and rvalue-qual overloads of Future::onError() currently have identical semantics; neither invalidates / moves-out its Future object.)

Reviewed By: yfeldblum

Differential Revision: D9510204

fbshipit-source-id: 5c1ff2f36c5d84c583268f5e952fd322ea8e9327
2018-08-25 18:21:25 -07:00
Marshall Cline
7ca1134ee4 use rvalue-qual Future::onError(): pass 3
Summary:
This is part of "the great r-valuification of folly::Future":

* This is something we should do for safety in general.
* Several of folly::Future's methods are lvalue-qualified even though they act as though they are rvalue-qualified, that is, they provide a postcondition that says, in effect, callers should act as though the method invalidated its `this` object (regardless of whether that invalidation was actual or logical).
* This violates the C++ principle to "Express ideas directly in code" (see Core Guidelines), and generally makes it more confusing for callers as well as hiding the actual semantics from tools (linters, compilers, etc.).
* This dichotomy and confusion has manifested itself by some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
* The goal of rvalueification is to make sure methods that are logically rvalue-qualified are actually rvalue-qualified, which forces callsites to acknowledge that rvalueification, e.g., `std::move(f).onError(...)` instead of `f.onError(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.

Codemod changes:

* expr.onError(...) ==> std::move(expr).onError(...)  // if expr is not already an xvalue
* expr->onError(...) ==> std::move(*expr).onError(...)

Note: operator precedence of that last step is safe - no need to parenthesize `expr`. Reason: `->` binds more tightly than unary `*`.

Reviewed By: yfeldblum

Differential Revision: D9505757

fbshipit-source-id: de666f3a877313526d10f5d3569a1bbb2203f066
2018-08-24 23:08:32 -07:00