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
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
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
Summary: We should only attempt to include selinux on Linux, not macOS.
Reviewed By: strager
Differential Revision: D14181109
fbshipit-source-id: be47b7bdadc3409577fa114559e905214848ebd8
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
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
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
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
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
Summary:
Now that the deadlock in ProcessNameCache has been fixed, bring it
back.
Reviewed By: strager
Differential Revision: D13742965
fbshipit-source-id: f407105e06b9954766bdb48ef1303e2003c07284
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
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
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
Summary: This just makes the debug a little easier to follow.
Reviewed By: chadaustin
Differential Revision: D13680020
fbshipit-source-id: e4045822e56ba42a831ccb0ceaa9baaba5b79a10
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
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
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
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
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
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
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
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
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
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
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
Summary: only include and use it on linux
Reviewed By: simpkins
Differential Revision: D13475715
fbshipit-source-id: 6b0b9da1b32088e01cbb932f9b3ed62532dfe00f
Summary: only add the selinux deps if we found selinux
Reviewed By: simpkins
Differential Revision: D13475711
fbshipit-source-id: c3375282b61881317f9a6c4c8e321ce717d1f9ab
Summary: overlooked because there is no CI exercising this today
Reviewed By: simpkins
Differential Revision: D13475721
fbshipit-source-id: 3e8fe280ab73d249da374129b37d32cd7e17f472
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
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
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
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
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
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
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
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
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
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
Summary: This option was added upstream and to our internal 4.16 kernel series
Reviewed By: chadaustin
Differential Revision: D12823476
fbshipit-source-id: 54d241b77eff92f4083c0f1d4c98b47a6a098e3f
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
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
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
Summary: Always send write requests straight to the inode rather than going through FileHandle.
Reviewed By: wez
Differential Revision: D10220619
fbshipit-source-id: 9ce328583cf0fa9d7d8850d92d9e15ddc382d6a3
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
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
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
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
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
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
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
Summary: I wanted to see more details from the fuse requests, so log them.
Reviewed By: strager
Differential Revision: D9944649
fbshipit-source-id: 143703528fa029ed51e6cb42a5f6d8b3b0230ca3
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
Summary: Begin tracking pids passed into FUSE in the ProcessAccessLog.
Reviewed By: strager
Differential Revision: D9595795
fbshipit-source-id: 02e5fefebcd0de860274409ba6258f14fa050b55
Summary: While looking at FuseChannel I noticed an unnecessary Future.
Reviewed By: strager
Differential Revision: D9595672
fbshipit-source-id: 5c84822c4f2c4c3c78b88456e44728e463d5a1e8
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
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
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
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
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
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).ensure(...)` instead of `f.ensure(...)`. This syntactic change in the callsites forces callers to acknowledge the method's rvalue semantics.
This diff started as a Codemod, then required manual fixes. Here were the codemod steps:
* expr.ensure(...) ==> std::move(expr).ensure(...) // if expr is not already an xvalue
* expr->ensure(...) ==> std::move(*expr).ensure(...)
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: D9332070
fbshipit-source-id: 882121fe82c05fdb196ce676db686b6bc254974b
Summary:
The setcon() failure is not actionable or interesting, so don't log it
to stdout at Eden startup.
Reviewed By: wez
Differential Revision: D9344467
fbshipit-source-id: 68435c8f22c228f2fbb86f37c2b1874723934169
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.
6/n: Codemod rvalue-future<T>.then(...) to rvalue-future<T>.then(...).
Reviewed By: yfeldblum
Differential Revision: D9152002
fbshipit-source-id: 166475c1dcafb29a11154cbfbdf7e2e1feaf745b
Summary:
Fixed a data race in Eden's `FuseChannel` implementation that could cause a crash if a `FUSE_INTERRUPT` request was concurrently on a different thread while a first thread was still launching the original request.
Modified `FakeFuse` to use `SOCK_SEQPACKET` instead of `SOCK_STREAM` so that tests can submit several requests in a pipelined fashion without having to first wait until the previous request was received. This requires the `recvResponse()` function to first read the header with `MSG_PEEK` to determine the response size and then subsequently read the entire message atomically to avoid reading a truncated message.
Added a new unit-test that exercises the `FUSE_INTERRUPT` race condition by sending a series of alternating `FUSE_LOOKUP`/`FUSE_INTERRUPT` requests.
Reviewed By: simpkins
Differential Revision: D9023654
fbshipit-source-id: 0eb44669ea8c4f58cf4313adf6ceb11098115a70
Summary:
I tried to build eden using GitHub version and ran into SELinux issues.
This patch fixed related issues, namely:
- `set(X Y)` sets X to string literal Y. Change it to `set(X ${Y})`.
- `SELINUX_INCLUDE_DIR` could be undefined. Check before use it.
- `./eden/fs/eden-config.h` and `./build/eden/fs/eden-config.h` both exist and
the former is used. Set include dirs so the latter gets used.
Reviewed By: bolinfest
Differential Revision: D9029272
fbshipit-source-id: 0c94bbe2f9e3fa90973802ddde16ad4d9ddfc0e8
Summary:
Many of the tests in FuseChannelTest.cpp perform blocking waits on Futures.
Previously they used a 100ms timeout. This could sometimes result in flaky
test failures if the system was under heavy CPU load. Bumping up the timeout
to 1 second appears to avoid this issue.
Reviewed By: lewissbaker
Differential Revision: D9024730
fbshipit-source-id: 7af78c61008c6b4c1e5e130b3d37b2f3ac787a01
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.
3/n: Codemod rvalue-future<T>.then(callable with operator()(not-a-try)) to rvalue-future<T>.thenValue(callable with operator()(not-a-try)).
Reviewed By: yfeldblum
Differential Revision: D8986716
fbshipit-source-id: 906339d9ffb90b3c38a24ce8bf0cef7be318d946
Summary:
This is part of "the great r-valuification of folly::Future":
* This is something we should do for safety in general.
* Context: `Future::get(...)` means both `Future::get()` and `Future::get(Duration)`
* Using lvalue-qualified `Future::get(...)` has caused 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.
* Problems with `Future::get(...) &`: it moves-out the result but doesn't invalidate the Future - the Future remains (technically) valid even though it actually is partially moved-out. Callers can subsequently access that moved-out result via things like `future.get(...)`, `future.result()`, `future.value()`, etc. - these access an already-moved-out result which is/can be surprising.
* Reasons `Future::get(...) &&` is better: its semantics are more obvious and user-testable. It moves-out the Future, leaving it with `future.valid() == false`.
Reviewed By: LeeHowes
Differential Revision: D8756764
fbshipit-source-id: 5c75c79cebcec77658a3a4605087716e969a376d
Summary:
This is part of "the great r-valuification of folly::Future":
* https://fb.facebook.com/groups/fbcode/permalink/1726007767436055/
* This is something we should do for safety in general.
* Using lvalue-qualified `Future::get()` has caused 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.
* Drill-down: `Future::get() &` moves-out the result but doesn't invalidate the Future - the Future remains (technically) valid even though it actually is partially moved-out. Callers can subsequently access that moved-out result via things like `future.get()`, `future.result()`, etc., which will cause surprising results - accessing an already-moved-out result. The semantics of `Future::get() &&` are more obvious: it moves-out the Future, leaving it with `future.valid() == false`.
Codemod steps (where `get(...)` means both `Future::get()` and `Future::get(Duration)`):
* expr.get(...) ==> std::move(expr).get(...) // if expr is not already an xvalue
* expr->get(...) ==> std::move(*expr).get(...)
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: D8625157
fbshipit-source-id: 70323ff3189631ce5597d3c9b17629c99f5d457b
Summary:
Update edenfs to fork the privhelper process as the first thing we do, before
calling folly::init(). This allows us to drop privileges before processing
command line arguments and doing any other startup work.
Reviewed By: wez
Differential Revision: D8212778
fbshipit-source-id: d67e3700305fdb01cb6188645b37875ceb53d21f
Summary:
Add a call to tell the privhelper process to redirect its log messages to a
different file handle.
This call will make it possible for the privhelper processed to be forked
before the main edenfs process has processed the --logPath argument. The main
edenfs process can now drop privileges before it processes arguments and opens
the log file, and it can then pass the file handle to the privhelper process.
Reviewed By: wez
Differential Revision: D8212776
fbshipit-source-id: 3ec6bfc82ee090216d66c5bfd1b8c2b8819c1f45
Summary:
Add a PrivHelper::detachEventBase() method, and rename PrivHelper::start() to
attachEventBase().
This makes it possible to detach a running PrivHelper from its EventBase and
re-attach it to an EventBase later to restart it. This will be useful in
upcoming diffs to allow performing calls to the PrivHelper before the main
thrift server EventBase has started.
Reviewed By: wez
Differential Revision: D8212777
fbshipit-source-id: d5a9bf672afa8b16e53201ac747d77337e1cc307
Summary:
[Folly] In Futures Core, destroy context when destroying callback since they basically go together.
Also removes expectations in an Eden FS test case which affirm the old behavior, along with a TODO to remove the expectations once the affirmed behavior is fixed.
Reviewed By: marshallcline
Differential Revision: D8347040
fbshipit-source-id: ed9fec932ad1e0aa1e40675cf70081d19bbe4325
Summary:
This updates the privhelper code to use the UnixSocket class for performing
I/O. This reduces the number of separate implementations of code we have for
sending file descriptors across Unix domain sockets, and also makes the
privhelper APIs non-blocking.
This will make it easier to clean up some of the initialization ordering in
the future. It will also make it easier to send file descriptors to the
privhelper server, instead of just receiving them. This may be helpful for
passing a file descriptor to use for logging to the privhelper process, which
will make it easier to fork the privhelper before logging redirection has
occurred.
Reviewed By: bolinfest
Differential Revision: D8053422
fbshipit-source-id: 1f8fdf22afc797eead0213be1352ea530762140d
Summary:
Up until now all of the privhelper APIs have been blocking calls. This
changes the privhelper functions to return Futures, and updates all users of
these APIs to be able to handle the results using Futures.
One benefit of this change is that all existing mount points are remounted in
parallel now during startup, rather than being mounted serially. The old code
performed a blocking `get()` call on the future returned by
`EdenServer::mount()`.
The privhelper calls themselves are still blocking for now--they block until
complete and always return completed Future objects. I will update the
privhelper code in a subsequent diff to actually make it asynchronous.
Reviewed By: bolinfest
Differential Revision: D8053421
fbshipit-source-id: 342d38697f67518f6ca96a37c12dd9812ddb151d
Summary:
1. Enabled a number of additional C++ compiler warnings in Eden.
2. Fixed warnings-turned-errors that resulted from this change.
Reviewed By: simpkins
Differential Revision: D8132543
fbshipit-source-id: 2290ffaaab55024d582e29201a1bcaa1152e6b3e
Summary:
Fix the code in FuseChannel::processSession() to look at the errno value
instead of the return value from `read()` when checking for ENODEV.
I accidentally broke this in D7436867. This wasn't causing any major issues,
since we would still break out of the loop, but it caused us to incorrectly
print warning messages about receiving an unexpected error. We would also
print this error message once per FUSE thread rather than just once for the
mount point.
Reviewed By: bolinfest
Differential Revision: D8109889
fbshipit-source-id: 9a53ca47b436ccf6731144ee2b829131339b6445
Summary: Now that permissions on directories work, verify umask works as intended.
Reviewed By: simpkins
Differential Revision: D7783743
fbshipit-source-id: 635221cd3255cc20e9ffa26b6838922c4a4110f3
Summary:
This is not at all clear from cppreference.com, but per
https://www.youtube.com/watch?v=dTeKf5Oek2c, it sounds to me like
recommended practice is to either:
`using namespace std::chrono_literals` (or string_literals or
whatever) to pull in a focused set of literals.
or
`using namespace std::literals` to pull in all standard literals.
or
`using namespace std` to pull in everything.
`using namespace std::literals::chrono_literals` is unnecessarily
verbose.
Adopt those standards in Eden.
Reviewed By: simpkins
Differential Revision: D8060944
fbshipit-source-id: 4d9dd4329698b7ff5e5c81b5b28780ca4d81a2a1
Summary:
I'd misunderstood the point of SharedMutex's upgrade locks -
unless they're used in rare paths, they don't allow for increased
concurrency. This diff and D7885245 remove all of Eden's ulocks,
replacing them with a helper which checks once with an rlock held, and
if the check fails, switches to a wlock (and checks again).
Reviewed By: yfeldblum
Differential Revision: D7886046
fbshipit-source-id: 545bb0dbb4898cbb71412efc6222ef12e4ee374e
Summary:
Update the folly::Init code to define a `--logging` command line flag, and call
`folly::initLoggingOrDie()` with the value of this command line during
initialization.
This is similar to the existing code that initializes the glog library.
(Programs can use both glog and folly logging together in the same program, and
I expect that many programs will do so as parts get converted to folly::logging
and parts remain using glog.)
Reviewed By: yfeldblum
Differential Revision: D7827344
fbshipit-source-id: 8aa239fbad43bc0b551cbe40cad7b92fa97fcdde
Summary:
Per yfeldblum's comment in D7886046, we can use folly::unit instead
of folly::Unit{}. We weren't using folly::unit anywhere, so this diff
replaces folly::Unit{} elsewhere in the Eden code.
Reviewed By: yfeldblum
Differential Revision: D7913462
fbshipit-source-id: fa6ab44ceb406d38713e0f4649224a74e6e51abd
Summary:
Promote the folly logging code out of the experimental subdirectory.
We have been using this for several months in a few projects and are pretty
happy with it so far.
After moving it out of the experimental/ subdirectory I plan to update
folly::Init() to automatically support configuring it via a `--logging` command
line flag (similar to the initialization it already does today for glog).
Reviewed By: yfeldblum, chadaustin
Differential Revision: D7755455
fbshipit-source-id: 052db34c97f7516728f7cbb1a5ad959def2f6efb
Summary:
To avoid bugs similar to the ones fixed in D7781691, mark a bunch of
folly::Future<folly::Unit> functions with FOLLY_NODISCARD.
Reviewed By: simpkins
Differential Revision: D7782224
fbshipit-source-id: 23ba42aa63011cc33e5a6e18d5bc6d00403a78d3
Summary:
Remove the EDEN_HAS_COMMON_STATS checks now that the common/stats stubs have
the required APIs needed by Eden.
Reviewed By: wez
Differential Revision: D7479593
fbshipit-source-id: cc3db50288bfea7aefd6c91391ab800628b7978f
Summary:
Add a macro to help users define the `getBaseLoggingConfig()` function.
While I would prefer to avoid macros if possible, this seems worthwhile. This
saves 4 or 5 lines of boilerplate code in each program that sets a custom base
logger setting. It also reduces the likelihood of a developer accidentally
having a typo in the function name, which would still build successfully but
not have the desired results.
Reviewed By: chadaustin
Differential Revision: D7457652
fbshipit-source-id: 1c316c7ea6949c16bd7b61c0440cc1ee69ecb83e
Summary:
Fix FuseChannel::processSession() to always process all FUSE requests that it
reads. Previously it checked to see if it should stop immediately after
reading FUSE request. It was possible for the old process to successfully read
a FUSE request, see that it was supposed to stop, and then exit this worker
thread without ever processing this FUSE request. This would cause the client
that sent the request to hang indefinitely, since no response would ever be
sent.
Reviewed By: wez
Differential Revision: D7436867
fbshipit-source-id: c58c2f6c49102fa6b66ac83fc1639595a5277ce0
Summary:
Update FuseChannel to send all invalidation requests in a separate thread.
This eliminates a deadlock that could previously occur during checkout
operations. The invalidation requests would block until they could acquire the
kernel's inode lock on the inode in question. However, the inode lock may
already be held by another thread attempting to perform an unlink() or rename()
call. These FUSE unlink or rename operations would be blocked waiting on
Eden's mount point rename lock, which was acquired by the checkout operation.
Checkout operations now let the invalidations complete asynchronously, but we
wait for all invalidation operations to complete before indicating to our
caller that the checkout has succeeded.
Reviewed By: chadaustin, wez
Differential Revision: D7404971
fbshipit-source-id: 6fa20c00d054e210eb0258d247d083010557f210
Summary:
Request data was coming back as null because of a race between ctx->getContextData() in FuseChannel::getOutstandingRequests() and the it initialization.
Also, added a check to verify rdata is valid before dereferencing it.
Plus, fixed debug.py where output gave an error.
Reviewed By: chadaustin
Differential Revision: D7377993
fbshipit-source-id: 8343119983c74185fd5d8cc05c2f5af63dcff99e
Summary:
The changes in this diff comments out unused parameters. All changes are automated using clang-tidy.
This will allow us to enable `-Wunused-parameter` as error.
Reviewed By: simpkins
Differential Revision: D7371610
fbshipit-source-id: 0134e2f0b916313d690c073a46d747c52399a226
Summary:
Set the default logging settings by overriding folly::getBaseLoggingConfig()
rather than by setting a default value for the `--logging` command line
argument.
This has two advantages:
- Custom logging settings supplied in the `--logging` argument will now
combined with the base logging settings rather than completely replacing
them. Previously users had to always add "eden=DBG2" to this argument if
they did not want to drop this setting.
- This will make it possible to move the definion of the `--logging` flag to
folly::Init and not lose our custom settings for Eden.
Reviewed By: yfeldblum
Differential Revision: D7348519
fbshipit-source-id: bd9012f387b0460da8bcc9c7c31aef46b1abec5c
Summary:
Fix the code to generate exceptions based on an errno error using
std::generic_category rather than std::system_category.
Reviewed By: yfeldblum
Differential Revision: D7329997
fbshipit-source-id: 3fe257bbbc7a631c801f31120592c8bdbc25c8bf
Summary:
Add a new utils/SystemError.h header with helper functions to check if a
`std::system_error` contains an errno value.
Most of the code in Eden previously only checked for `std::system_category`
when looking for errno values. `std::generic_category` is the correct category
to use for errno exceptions, but folly/Exception.h incorrectly throws them as
`std::system_category` today. This change makes Eden treat either error type
as errno values for now.
Reviewed By: yfeldblum
Differential Revision: D7329999
fbshipit-source-id: 67a3c3ea10371c53a2e34236b7575deac4cbd53a
Summary:
Added a thrift call to return the outstanding FUSE requests.
Cli will call the thrift and print the output.
Added a unit test to test getOutstandingRequests().
Reviewed By: simpkins
Differential Revision: D7314584
fbshipit-source-id: 420790405babdb734f598e19719b487096ec53ca
Summary:
This test exercises code under eden/fs/fuse, so put it with that directory's
tests.
Reviewed By: chadaustin
Differential Revision: D7314457
fbshipit-source-id: b664aaa3086b2e65f7ae8b76ae71878f39fd9d17
Summary:
Previously the FuseChannel code unconditionally set `connInfo_.major` and
`connInfo_.minor` to the FUSE version that it supports, rather than the FUSE
version supported by the kernel it is talking to.
This does not appear to be the intended behavior, given that in several other
location it checks the value of `connInfo_->minor` to see if particular
features are supported.
Reviewed By: wez
Differential Revision: D7300845
fbshipit-source-id: 1f5946134037b2cbe03a8b86241acaa401c21ffa
Summary:
Remove the `FuseChannel::stealFuseDevice()` method, and instead change the
session completion future to return the FUSE device FD if it is still valid.
This ensures we only extract the FUSE device when it is still valid: if an
unmount event is being processed by a FUSE worker thread simultaneously with a
call to `FuseChannel::takeoverStop()` the session completion future will return
an empty `folly::File` since the FUSE device has been shut down.
This also helps clarify the synchronization semantics around modification to
the `fuseDevice_` member variable.
Reviewed By: wez
Differential Revision: D7300847
fbshipit-source-id: 02ced8fc9d24e7cd526d911782949d0bfbf0e5a7
Summary:
Eliminate a few unnecessary includes from Dispatcher.h and one from
RequestData.h
Reviewed By: wez
Differential Revision: D7300846
fbshipit-source-id: 0cde1f70f605fcb5c95ed13950f4fa8d353fd72f
Summary:
FuseChannel::processSession() was missing a return statement after we received
EOF from the FUSE device. This would cause us to fall through and try
processing the empty buffer as a request. This would fail, and we would try to
send a reply, which would in turn fail since the FUSE device was closed.
This was only an issue with the unit tests: on real FUSE devices we get an
ENODEV error when reading from the device. In unit tests we get an EOF
instead.
Reviewed By: wez
Differential Revision: D7299336
fbshipit-source-id: c275e03edbf594c0d120257b1cfc89d7c75f60f6
Summary:
The FuseChannel destructor previously could be invoked while there are still
outstanding requests, which would cause problems when responses were generated
for these requests.
This makes the FuseChannel destructor private, requiring users to always
allocate FuseChannel objects on the heap and destroy them using a destroy()
method. The destroy() method still blocks waiting for the FUSE worker threads
to exit, but if there are still outstanding FUSE requests it defers the
deletion of the FuseChannel until they are complete.
The EdenMount class currently couldn't ever destroy FuseChannel objects with
outstanding requests, since it waits for the completion future to fire before
invoking the destructor. Fixing this behavior is mostly for correctness of the
FuseChannel API and for the benefit of our unit tests.
Reviewed By: chadaustin
Differential Revision: D7297829
fbshipit-source-id: 643d1332be84a1f25ee30ba2a2ea3515806f00ef
Summary:
Add a simple test program that starts a FUSE mount. At the moment the only
call it actually responds to is getattr() for the root inode. However this is
sufficient for exercising the FuseChannel startup and shutdown code.
Reviewed By: chadaustin
Differential Revision: D7297827
fbshipit-source-id: 1bae6015def37913a5bf57541a14c3c8c050eff5
Summary:
Remove the `FuseChannel::getSessionCompleteFuture()` method and instead have
`initialize()` and `initializeFromTakeover()` return the `Future` that can be
used to wait on session completion.
This makes it explicit from an API perspective that this session completion
future will only be invoked if initialization is successful. It also
eliminates the possibility of anyone calling getSessionCompleteFuture() more
than once.
I also changed the session completion future to use a folly::SemiFuture rather
than a folly::Future, to make it explicit that callers generally should not
execute their callback inline in the same thread where the SemiFuture is
fulfilled.
Reviewed By: chadaustin
Differential Revision: D7297835
fbshipit-source-id: 3a1157951f0738f1692833ed5e875c3e9c6d6d69
Summary: Testing convenience. The NO_CHECK constructor is no longer necessary now that simpkins made the original constructor constexpr.
Reviewed By: simpkins
Differential Revision: D7295539
fbshipit-source-id: 717f5a7a8d3d9790966fad441da7b3308495e2c3
Summary:
The FuseChannel unit tests would sometimes hang because a worker thread would
still be stuck in a blocking read() call.
The issue appears to be that the kernel would often automatically restart the
interrupted read(), so the worker would never wake up. This switches the code
from using SIGPIPE to wake up, and instead uses SIGUSR2, and installs a handler
for SIGUSR2 with the SA_RESTART flag unset.
It's possible that this was an issue only for the unit tests, where the FUSE
device is a socket. I'm not sure if the kernel would automatically restart
read operations on a real FUSE device.
There are still theoretically some race conditions here, since
`requestSessionExit()` could be called right after a worker thread checks
`(runState_` and before it enters the blocking `read()` call. Fixing that
would likely require switching to something like `folly::EventBase` or manually
using epoll. Fortunately I haven't seen that be an issue in practice so far.
Reviewed By: chadaustin, wez
Differential Revision: D7282002
fbshipit-source-id: 99d29de13a7858dd25f258fdc94d8f8c71ee84d9
Summary:
This changes FuseChannel to fulfill the session complete future immediately in
the thread where it finishes. This may occur inside a FUSE worker thread.
It is now up to the caller to use `via()` if they want their callback to run in
a separate thread.
This simplifies the FuseChannel code and also addresses a crash that occurs in
the unit tests sometimes: If the FuseChannel destructor is invoked without
explicitly invoking the FuseChannel, it would schedule the session complete
promise to be fulfilled in a separate EventBase thread. However, the promise
may then have already been destroyed by the time it can run in the EventBase
thread.
There are still additional synchronization issues to fix in the FuseChannel
tests, but this resolves one problem.
Reviewed By: chadaustin
Differential Revision: D7282001
fbshipit-source-id: be64d3ef6a0e664ed7a2cf93a549acc140184095
Summary:
If you run `eden daemon` on a machine where sudo needs input, sudo's
output would get redirected to edenfs.log and eden daemon would appear
to hang. Worse, if you pressed ctrl-C, sudo would remain in the
background and continue to swallow keypresses from the tty, appearing
to somewhat break your shell until it gave up. The fix is to stop
redirecting stdout and stderr from Python and instead have edenfs
redirect itself into the log if given.
Reviewed By: simpkins
Differential Revision: D7223399
fbshipit-source-id: bae17b150b6594a3dd87bb88741c8dcefd1c5213
Summary:
Update FuseChannel::startWorkerThreads() to return without doing anything if
runState_ indicates that the FuseChannel is currently stopping.
This addresses a race betwen startWorkerThreads() and the destructor. If the
destructor is invoked in parallel with startWorkerThreads() the destructor can
grab the state_ lock and clear out the workerThreads list first. This then
causes problems when startWorkerThreads() runs and adds new threads to the
workerThreads list, which then never get joined.
Reviewed By: wez
Differential Revision: D7253574
fbshipit-source-id: f2cac11f1e71e1a14e3f020368152e0f2948c625
Summary:
Update FuseChannel::readInitPacket() to honor `runState_` and stop if it is
changed to indicate that the channel is stopping. Previously the init worker
thread would always wait until it received an INIT packet or an error on the
FUSE channel.
This also refactors the `readInitPacket()` code slightly mainly to to reduce
the level of indentation.
Reviewed By: wez
Differential Revision: D7253575
fbshipit-source-id: 7f27698947b629daecd9cd4ddffb6e6a921ce41e
Summary:
Update the FuseChannel code to keep track of why it is stopping, and to return
this data in the session complete future.
This is mainly just available for diagnostic reasons, and at the moment nothing
is using this outside of the unit tests.
Reviewed By: wez
Differential Revision: D7253573
fbshipit-source-id: 0c7e5b8bd9c3c1de3788918c2257c06d4fe0a90c
Summary:
Catch exceptions thrown by FuseChannel::processSession() and terminate the
FuseChannel if any errors occur.
Also explicitly mark `fuseWorkerThread()` and `initWorkerThread()` as
`noexcept` so we will crash with useful exception backtraces if an unhandled
exception does ever attempt to propagate out of these functions. (Prior to
gcc 8.x, `std::thread()` ends up swallowing the exception backtrace information
if a thread function throws an exception.)
Reviewed By: chadaustin
Differential Revision: D7243909
fbshipit-source-id: fbe7173a2167532d7e42901f810c17bb173d8d63
Summary:
- Update FuseChannel::initializate() to not require an Executor. Rather than
performing initialization using the supplied Executor, it simply starts one
worker thread first and performs initialization there.
- Change the API semantics slightly so that the session complete future is
invoked only if initialization succeeds. Previously the session complete
future could be invoked if initialization failed as well.
- Replace the activeThreads counter with a stoppedThreads counter to determine
when the session complete future should be invoked. The previous
activeThreads behavior was somewhat racy, since the activeThreads counter was
incremented inside each worker thread, but most places that checked it did
not wait to ensure that all worker threads had successfully incremented it
yet.
Reviewed By: chadaustin
Differential Revision: D7243910
fbshipit-source-id: 93b3465509bd9bf6fa90ea097e70dac3193172f9
Summary:
Split up the FuseChannel and EdenMount APIs for starting a brand new FUSE
channel vs taking over an existing channel.
These operations are somewhat different as initializing a new FUSE channel
requires performing a negotiation with the kernel that will not complete
immediately. Therefore these APIs return a Future object. Taking over an
existing FUSE channel is always an immediate operation, and therefor does not
need to return a Future object.
Reviewed By: chadaustin
Differential Revision: D7241997
fbshipit-source-id: 22780f2df76b2d554ab2a4043b6425fa7a4e9c94
Summary:
Update FuseChannel to only join its worker threads in its destructor, rather
scheduling a function in the main EventBase thread to join each thread as soon
as it completes.
This makes FuseChannel a bit easier to use: the destructor can now be called at
any point in time (as long as it is not called inside one of the worker threads
themselves). If it is called while the FuseChannel is still running this will
automatically stop the worker threads.
Reviewed By: wez
Differential Revision: D7224369
fbshipit-source-id: 4de0e7c0c677e870fd629eaaaa5ab40d134ee870
Summary:
Ensure that we ignore SIGPIPE in unit tests. The
`FuseChannel::requestSessionExit()` function sends SIGPIPE to its worker
threads, and expects this to wake up the worker threads if they are stuck
inside a read() call.
In normal program builds folly/Subprocess.cpp causes SIGPIPE to be ignored on
startup. However, Subprocess.cpp is not necessarily linked into all of our
unit tests. I haven't tracked down exactly why, but SIGPIPE in the unit tests
seems to kill the entire process only in opt-ubsan builds.
In the future we probably should clean up how requestSessionExit() triggers its
worker threads to exit: the current code is still potentially racy, since the
worker threads could receive the SIGPIPE just after checking `sessionFinished_`
and before calling `read()`, in which case they may go back to sleep rather
than exiting. However for now this should be a simple fix to get the tests
passing in all build modes.
Reviewed By: wez
Differential Revision: D7224370
fbshipit-source-id: a56fe3331fc5aa6a49ccbe6b0678b479a44ffc07
Summary:
Only the FuseChannel code should invoke the handler methods. This makes them
all private.
This did require making handlerMap be a private static member variable of
FuseChannel so that it can access these methods during its initialization.
Reviewed By: wez
Differential Revision: D7126002
fbshipit-source-id: 28c421173cb6cf8b7a8e1ca085ad78ec9ea76b8f
Summary: This method is never removed anywhere, so delete it for now.
Reviewed By: wez
Differential Revision: D7126003
fbshipit-source-id: 974645c8063fc17c4e69e5e46c870e52fecfcd65
Summary:
We were curious why we needed `sudo gdb` to debug Eden even
though it drops privileges. The process dumpable bit is the trick.
Reviewed By: simpkins
Differential Revision: D7182742
fbshipit-source-id: 48e3dd778bf07ec1bfadace4edeb64668b936b9b
Summary:
Today, attaching gdb to edenfs requires sudo. Our hypothesis is that,
even though we drop uid and gid, the SELinux context remains elevated,
preventing ptrace from attaching.
Reviewed By: simpkins
Differential Revision: D7128692
fbshipit-source-id: 0f9b9a2ebef5a438bf0f4dcdc00a7b703a7aea02
Summary:
`DCHECK_NE()` unfortunately is not constexpr, so it results in a build error
when used in a constexpr function. Fortunately `assert()` is allowed in
constexpr functions as of C++14, so use it instead.
Reviewed By: wez
Differential Revision: D7108887
fbshipit-source-id: 2be9181929a357c23f48c09173646683060aad28
Summary:
This adds a FakeFuse class to simulate a FUSE connection to the kernel, but
that is actually communicating with our userspace test harness code. This also
adds a FakePrivHelper class which implements the PrivHelper interface but
returns FakeFuse connections rather than real FUSE connections to the kernel.
This will enable us to write unit tests that exercise more of the FUSE and
EdenMount logic, and will also enable us to test error conditions and ordering
behaviors that are difficult to reliably reproduce with a real FUSE mount.
This also includes some very basic tests using this new code. The code in
fuse/test/FuseChannelTest.cpp creates a FuseChannel using a FakeFuse object,
and the code in inodes/test/FuseTest.cpp creates a full EdenMount object using
FakePrivHelper and FakeFuse. The tests are pretty similar for now, and only
exercise the FUSE initialization code. I will expand these tests in subsequent
diffs.
Reviewed By: wez
Differential Revision: D7050826
fbshipit-source-id: 4f82375d65664ca3a5a228a577caa4d1d47454fe
Summary:
I'm seeing test failures that I have not yet understood and I
thought they might be caused by an implicit conversion from
fusell::InodeNumber to bool. Well, they're not, but this is how I
discovered that. I'm not sure I want to land this change, but I'm
going to leave it around until I figure out what's happening with my
other diffs.
Reviewed By: simpkins
Differential Revision: D7077635
fbshipit-source-id: 50bf67026d2d0da0220c4709e3db24d841960f4b
Summary:
This removes the TARGETS files from the eden github repository. The
open source buck build has been failing for several months, since buck
removed support for the thrift_library() rule.
I will potentially take a stab at adding CMake build support for Eden
at some point in the future.
Reviewed By: chadaustin
Differential Revision: D6893233
fbshipit-source-id: e6023094a807cf481ac49998c6f21b213be6c288
Summary:
Move all of the privhelper functionality into a PrivHelper class. The
ServerState object now stores the PrivHelper object to use, rather than having
a global singleton.
This will make it easier to stub out the PrivHelper functionality during unit
tests.
Reviewed By: wez
Differential Revision: D6929862
fbshipit-source-id: e3edcb0a03ba9afdf34554cb961fd74557cdd6e3
Summary:
We no longer need this and I believe that this was contributing
to this source of flakiness in our CI; this stack trace triggers when
we get an aborted or short read from the kernel:
```
*** Aborted at 1517827659 (Unix time, try 'date -d 1517827659') ***
*** Signal 11 (SIGSEGV) (0x2d0) received by PID 153706 (pthread TID 0x7f63ea37d700) (linux TID 160573) (maybe from PID 720, UID 0) (code: address not mapped to object), stack trace: ***
@ 0000000002abfa2d folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
./folly/experimental/symbolizer/SignalHandler.cpp:413
@ 00007f646e96dacf (unknown)
@ 00007f646e96aa20 __pthread_kill
@ 0000000001873748 facebook::eden::fusell::FuseChannel::requestSessionExit()
./eden/fs/fuse/FuseChannel.cpp:483
@ 000000000187d2f7 facebook::eden::fusell::FuseChannel::processSession()
./eden/fs/fuse/FuseChannel.cpp:656
@ 000000000187dab9 facebook::eden::fusell::FuseChannel::fuseWorkerThread(unsigned long)
./eden/fs/fuse/FuseChannel.cpp:493
@ 00007f646f267170 execute_native_thread_routine
@ 00007f646e9637a8 start_thread
@ 00007f646e047a7c __clone
```
my theory is that we're allowing shutdownImpl to free things out from under other
threads before they've all seen this signal and wound down fully. This is slightly
speculative in that I haven't managed to reproduce this stack trace on my devserver.
We don't really need this additional signal any longer.
Reviewed By: simpkins
Differential Revision: D6907734
fbshipit-source-id: 0f0138b631a7201fc9a4a1c93c2cde846e869cbd
Summary:
Change `LoggerDB::get()` to a reference instead of a pointer since this
function can never return null.
Reviewed By: yfeldblum
Differential Revision: D6893206
fbshipit-source-id: af47063918a79c851fd39b838d6c63755166e033
Summary: We don't think this is happening, but let's test it!
Reviewed By: simpkins
Differential Revision: D6888038
fbshipit-source-id: 754b2ec8f78ff513fd350a74505915e2a1e9ba3e
Summary:
This diff moves the mount-time initialization handling
out of the main loop. This rationale for this is:
* We don't (and shouldn't!) need to process FUSE_INIT for takeover
processing, and this structure allows us to make stronger assertions
about our state.
* we can avoid spinning up multiple threads in the (rare!) case that
the FUSE_INIT fails
* It is now a little harder for exceptions during initialization to
escape our notice.
In rearranging this stuff, I found a race condition in the worker thread
shutdown; we could erroneously emit a completion event before all of
the threads had been torn down and this resulted in sporadic integration
test failures hitting the assertion for the number of joined threads
in the destructor.
Reviewed By: simpkins
Differential Revision: D6766330
fbshipit-source-id: 32afb5a7c739c75aebfdb0a8f896eec5f41ad33f
Summary:
Added comments linking to the task with discussion of why we are postponing
proper handling of umask and permissions in general for mkdir and mknod.
Reviewed By: wez
Differential Revision: D6808696
fbshipit-source-id: a6d74ea08448c8578282dc146ac81da187d4409c
Summary: This makes it easier to separate out the fuse initialization logic in a follow on diff.
Reviewed By: simpkins
Differential Revision: D6766329
fbshipit-source-id: 178d97e2302bc283c701039494e4477285abc0af
Summary:
This slightly improves the code in EdenMount by having
just a single call to the FuseChannel constructor with an optional
`fuse_init_out` instance that we can use to determine if we are
performing a takeover or not.
Reviewed By: simpkins
Differential Revision: D6766328
fbshipit-source-id: ece140a1572e2934a3e35bfe25b83af910346c18
Summary: Replaced memset used to 0-init structs with POD fields with = {} init.
Reviewed By: simpkins
Differential Revision: D6770868
fbshipit-source-id: 190205a277e3c73711813bc515b860b107d41f83
Summary:
This is moving some files around in preparation for
moving TakeoverData to using thrift for its serialization
Reviewed By: simpkins
Differential Revision: D6733405
fbshipit-source-id: 235ba237546f8ef606de8445db45683ce38a2d2c
Summary: Minor refactor replacing if with switch and a loop control comparison.
Reviewed By: wez
Differential Revision: D6730849
fbshipit-source-id: 69ac7be9fae3be6274b56dd2baea433ed2437f7c
