From 307a25e13cf595f6046fba6458f136f57c64a5ec Mon Sep 17 00:00:00 2001 From: Yedidya Feldblum Date: Wed, 12 Oct 2022 12:01:08 -0700 Subject: [PATCH] migrate constructions of folly::exception_wrapper Summary: It is no longer necessary to pass a reference to the caught exception along with `std::current_exception()`. Reviewed By: xavierd Differential Revision: D40252106 fbshipit-source-id: 08bfbea675c60097050f3f026af308fcf0a240df --- eden/fs/fuse/FuseChannel.cpp | 9 +++--- eden/fs/inodes/CheckoutAction.cpp | 45 ++++++++++++++--------------- eden/fs/inodes/CheckoutAction.h | 2 +- eden/fs/inodes/InodeMap.cpp | 7 ++--- eden/fs/inodes/Overlay.cpp | 8 ++--- eden/fs/inodes/TreeInode.cpp | 4 +-- eden/fs/service/EdenServer.cpp | 17 ++++++----- eden/fs/store/hg/HgBackingStore.cpp | 4 +-- eden/fs/takeover/TakeoverServer.cpp | 9 +++--- eden/fs/utils/ImmediateFuture-inl.h | 4 +-- eden/fs/utils/UnixSocket.cpp | 26 ++++++++--------- 11 files changed, 65 insertions(+), 70 deletions(-) diff --git a/eden/fs/fuse/FuseChannel.cpp b/eden/fs/fuse/FuseChannel.cpp index 4bb3a72c96..eee834e210 100644 --- a/eden/fs/fuse/FuseChannel.cpp +++ b/eden/fs/fuse/FuseChannel.cpp @@ -1229,12 +1229,11 @@ void FuseChannel::initWorkerThread() noexcept { // Start the other FUSE worker threads. startWorkerThreads(); - } catch (const std::exception& ex) { - XLOG(ERR) << "Error performing FUSE channel initialization: " - << exceptionStr(ex); + } catch (...) { + auto ew = folly::exception_wrapper(std::current_exception()); + XLOG(ERR) << "Error performing FUSE channel initialization: " << ew; // Indicate that initialization failed. - initPromise_.setException( - folly::exception_wrapper(std::current_exception(), ex)); + initPromise_.setException(std::move(ew)); return; } diff --git a/eden/fs/inodes/CheckoutAction.cpp b/eden/fs/inodes/CheckoutAction.cpp index bece993ff0..049ca7785b 100644 --- a/eden/fs/inodes/CheckoutAction.cpp +++ b/eden/fs/inodes/CheckoutAction.cpp @@ -127,10 +127,9 @@ Future CheckoutAction::run( }) .semi() .via(&folly::QueuedImmediateExecutor::instance()) - .thenError( - [rc = LoadingRefcount(this)](const exception_wrapper& ew) { - rc->error("error getting old tree", ew); - }); + .thenError([rc = LoadingRefcount(this)](exception_wrapper&& ew) { + rc->error("error getting old tree", std::move(ew)); + }); } else { store->getBlobSha1(oldEntry.second.getHash(), ctx->getFetchContext()) .thenValue([rc = LoadingRefcount(this)](Hash20 oldBlobSha1) { @@ -138,10 +137,9 @@ Future CheckoutAction::run( }) .semi() .via(&folly::QueuedImmediateExecutor::instance()) - .thenError( - [rc = LoadingRefcount(this)](const exception_wrapper& ew) { - rc->error("error getting old blob Sha1", ew); - }); + .thenError([rc = LoadingRefcount(this)](exception_wrapper&& ew) { + rc->error("error getting old blob Sha1", std::move(ew)); + }); } } @@ -156,10 +154,9 @@ Future CheckoutAction::run( }) .semi() .via(&folly::QueuedImmediateExecutor::instance()) - .thenError( - [rc = LoadingRefcount(this)](const exception_wrapper& ew) { - rc->error("error getting new tree", ew); - }); + .thenError([rc = LoadingRefcount(this)](exception_wrapper&& ew) { + rc->error("error getting new tree", std::move(ew)); + }); } else { // We don't actually compare the new blob to anything, so we don't need // to fetch it. This just marks that the new inode will be a file. @@ -174,13 +171,14 @@ Future CheckoutAction::run( .thenValue([rc = LoadingRefcount(this)](InodePtr inode) { rc->setInode(std::move(inode)); }) - .thenError([rc = LoadingRefcount(this)](const exception_wrapper& ew) { - rc->error("error getting inode", ew); + .thenError([rc = LoadingRefcount(this)](exception_wrapper&& ew) { + rc->error("error getting inode", std::move(ew)); }); } - } catch (const std::exception& ex) { - exception_wrapper ew{std::current_exception(), ex}; - refcount->error("error preparing to load data for checkout action", ew); + } catch (...) { + auto ew = exception_wrapper{std::current_exception()}; + refcount->error( + "error preparing to load data for checkout action", std::move(ew)); } return promise_.getFuture(); @@ -217,10 +215,9 @@ void CheckoutAction::setInode(InodePtr inode) { void CheckoutAction::error( folly::StringPiece msg, - const folly::exception_wrapper& ew) { - XLOG(ERR) << "error performing checkout action: " << msg << ": " - << folly::exceptionStr(ew); - errors_.push_back(ew); + folly::exception_wrapper&& ew) { + XLOG(ERR) << "error performing checkout action: " << msg << ": " << ew; + errors_.push_back(std::move(ew)); } void CheckoutAction::allLoadsComplete() noexcept { @@ -233,9 +230,9 @@ void CheckoutAction::allLoadsComplete() noexcept { doAction().thenTry([this](folly::Try&& t) { this->promise_.setTry(std::move(t)); }); - } catch (const std::exception& ex) { - exception_wrapper ew{std::current_exception(), ex}; - promise_.setException(ew); + } catch (...) { + auto ew = exception_wrapper{std::current_exception()}; + promise_.setException(std::move(ew)); } } diff --git a/eden/fs/inodes/CheckoutAction.h b/eden/fs/inodes/CheckoutAction.h index 989758d855..fd383de07c 100644 --- a/eden/fs/inodes/CheckoutAction.h +++ b/eden/fs/inodes/CheckoutAction.h @@ -117,7 +117,7 @@ class CheckoutAction { void setNewTree(std::shared_ptr tree); void setNewBlob(); void setInode(InodePtr inode); - void error(folly::StringPiece msg, const folly::exception_wrapper& ew); + void error(folly::StringPiece msg, folly::exception_wrapper&& ew); void allLoadsComplete() noexcept; bool ensureDataReady() noexcept; diff --git a/eden/fs/inodes/InodeMap.cpp b/eden/fs/inodes/InodeMap.cpp index c18e2289b9..e6b58c40ef 100644 --- a/eden/fs/inodes/InodeMap.cpp +++ b/eden/fs/inodes/InodeMap.cpp @@ -497,10 +497,9 @@ InodeMap::PromiseVector InodeMap::inodeLoadComplete(InodeBase* inode) { } mount_->publishInodeTraceEvent(std::move(endLoadEvent.value())); return promises; - } catch (const std::exception& ex) { - XLOG(ERR) << "error marking inode " << number - << " loaded: " << folly::exceptionStr(ex); - auto ew = folly::exception_wrapper{std::current_exception(), ex}; + } catch (...) { + auto ew = folly::exception_wrapper{std::current_exception()}; + XLOG(ERR) << "error marking inode " << number << " loaded: " << ew; for (auto& promise : promises) { promise.setException(ew); } diff --git a/eden/fs/inodes/Overlay.cpp b/eden/fs/inodes/Overlay.cpp index 4eaf7aeee9..98b925d3c7 100644 --- a/eden/fs/inodes/Overlay.cpp +++ b/eden/fs/inodes/Overlay.cpp @@ -187,11 +187,11 @@ folly::SemiFuture Overlay::initialize( std::move(mountPath), progressCallback, lookupCallback); - } catch (std::exception& ex) { + } catch (...) { + auto ew = folly::exception_wrapper{std::current_exception()}; XLOG(ERR) << "overlay initialization failed for " << localDir_ << ": " - << ex.what(); - promise.setException( - folly::exception_wrapper(std::current_exception(), ex)); + << ew; + promise.setException(std::move(ew)); return; } promise.setValue(); diff --git a/eden/fs/inodes/TreeInode.cpp b/eden/fs/inodes/TreeInode.cpp index 8d27084d2b..c7c3be8758 100644 --- a/eden/fs/inodes/TreeInode.cpp +++ b/eden/fs/inodes/TreeInode.cpp @@ -673,12 +673,12 @@ Future> TreeInode::startLoadingInodeNoThrow( // startLoadingInode() and convert any thrown exceptions into Future. try { return startLoadingInode(entry, name, fetchContext); - } catch (const std::exception& ex) { + } catch (...) { // It's possible that makeFuture() itself could throw, but this only // happens on out of memory, in which case the whole process is pretty much // hosed anyway. return makeFuture>( - folly::exception_wrapper{std::current_exception(), ex}); + folly::exception_wrapper{std::current_exception()}); } } diff --git a/eden/fs/service/EdenServer.cpp b/eden/fs/service/EdenServer.cpp index e18884f58b..c1a77261e0 100644 --- a/eden/fs/service/EdenServer.cpp +++ b/eden/fs/service/EdenServer.cpp @@ -620,11 +620,12 @@ Future EdenServer::stopMountsForTakeover( return std::move(takeover); }); })); - } catch (const std::exception& ex) { + } catch (...) { + auto ew = folly::exception_wrapper{std::current_exception()}; XLOG(ERR) << "Error while stopping \"" << mountPath - << "\" for takeover: " << folly::exceptionStr(ex); - futures.push_back(makeFuture>( - folly::exception_wrapper(std::current_exception(), ex))); + << "\" for takeover: " << ew; + futures.push_back( + makeFuture>(std::move(ew))); } } } @@ -1101,14 +1102,14 @@ std::vector> EdenServer::prepareMounts( folly::dynamic dirs = folly::dynamic::object(); try { dirs = CheckoutConfig::loadClientDirectoryMap(edenDir_.getPath()); - } catch (const std::exception& ex) { + } catch (...) { + auto ew = folly::exception_wrapper{std::current_exception()}; incrementStartupMountFailures(); logger->warn( "Could not parse config.json file: ", - ex.what(), + ew.what(), "\nSkipping remount step."); - mountFutures.emplace_back( - folly::exception_wrapper(std::current_exception(), ex)); + mountFutures.emplace_back(std::move(ew)); return mountFutures; } diff --git a/eden/fs/store/hg/HgBackingStore.cpp b/eden/fs/store/hg/HgBackingStore.cpp index 29ba089ffe..e10b3e8247 100644 --- a/eden/fs/store/hg/HgBackingStore.cpp +++ b/eden/fs/store/hg/HgBackingStore.cpp @@ -299,8 +299,8 @@ HgBackingStore::fetchTreeFromHgCacheOrImporter( // Data for this tree was not present locally. // Fall through and fetch the data from the server below. if (!FLAGS_hg_fetch_missing_trees) { - auto ew = folly::exception_wrapper(std::current_exception()); - return folly::makeFuture>(ew); + auto ew = folly::exception_wrapper{std::current_exception()}; + return folly::makeFuture>(std::move(ew)); } return fetchTreeFromImporter( manifestNode, edenTreeID, std::move(path), std::move(writeBatch)); diff --git a/eden/fs/takeover/TakeoverServer.cpp b/eden/fs/takeover/TakeoverServer.cpp index 037a6be4b8..03fb8b3170 100644 --- a/eden/fs/takeover/TakeoverServer.cpp +++ b/eden/fs/takeover/TakeoverServer.cpp @@ -186,9 +186,8 @@ Future TakeoverServer::ConnHandler::start() noexcept { return sendTakeoverData(std::move(data.value())); } })); - } catch (const std::exception& ex) { - return makeFuture( - folly::exception_wrapper{std::current_exception(), ex}); + } catch (...) { + return makeFuture(folly::exception_wrapper{std::current_exception()}); } } @@ -260,8 +259,8 @@ Future TakeoverServer::ConnHandler::sendTakeoverData( for (auto& file : msg.files) { XLOG(DBG7) << "sending fd for takeover: " << file.fd(); } - } catch (const std::exception& ex) { - auto ew = folly::exception_wrapper{std::current_exception(), ex}; + } catch (...) { + auto ew = folly::exception_wrapper{std::current_exception()}; data.takeoverComplete.setException(ew); return socket_.send( TakeoverData::serializeError(protocolCapabilities_, ew)); diff --git a/eden/fs/utils/ImmediateFuture-inl.h b/eden/fs/utils/ImmediateFuture-inl.h index 4692130f40..b7169cbbb6 100644 --- a/eden/fs/utils/ImmediateFuture-inl.h +++ b/eden/fs/utils/ImmediateFuture-inl.h @@ -115,9 +115,9 @@ makeImmediateFutureFromImmediate(Func&& func, Args... args) { } else { return func(std::forward(args)...); } - } catch (std::exception& ex) { + } catch (...) { return folly::Try( - folly::exception_wrapper(std::current_exception(), ex)); + folly::exception_wrapper{std::current_exception()}); } } } // namespace detail diff --git a/eden/fs/utils/UnixSocket.cpp b/eden/fs/utils/UnixSocket.cpp index 456079bf52..503edda59b 100644 --- a/eden/fs/utils/UnixSocket.cpp +++ b/eden/fs/utils/UnixSocket.cpp @@ -204,8 +204,8 @@ void UnixSocket::connect( auto connector = new Connector(callback, eventBase, std::move(socketFile)); connector->start(timeout); return; - } catch (const std::exception& ex) { - auto ew = exception_wrapper{std::current_exception(), ex}; + } catch (...) { + auto ew = exception_wrapper{std::current_exception()}; return callback->connectError(std::move(ew)); } } @@ -354,9 +354,9 @@ void UnixSocket::send(Message&& message, SendCallback* callback) noexcept { SendQueuePtr queueEntry; try { queueEntry = createSendQueueEntry(std::move(message), callback); - } catch (const std::exception& ex) { - auto ew = exception_wrapper{std::current_exception(), ex}; - XLOG(ERR) << "error allocating a send queue entry: " << ew.what(); + } catch (...) { + auto ew = exception_wrapper{std::current_exception()}; + XLOG(ERR) << "error allocating a send queue entry: " << ew; callback->sendError(make_exception_wrapper( "cannot send a message on a closed UnixSocket")); return; @@ -381,10 +381,10 @@ void UnixSocket::send(Message&& message, SendCallback* callback) noexcept { try { trySend(); - } catch (const std::exception& ex) { - auto ew = exception_wrapper{std::current_exception(), ex}; - XLOG(ERR) << "unix socket error during send(): " << ew.what(); - socketError(ew); + } catch (...) { + auto ew = exception_wrapper{std::current_exception()}; + XLOG(ERR) << "unix socket error during send(): " << ew; + socketError(std::move(ew)); } } } @@ -1014,10 +1014,10 @@ void UnixSocket::handlerReady(uint16_t events) noexcept { if (events & EventHandler::WRITE) { trySend(); } - } catch (const std::exception& ex) { - auto ew = exception_wrapper{std::current_exception(), ex}; - XLOG(ERR) << "unix socket I/O handler error: " << ew.what(); - socketError(ew); + } catch (...) { + auto ew = exception_wrapper{std::current_exception()}; + XLOG(ERR) << "unix socket I/O handler error: " << ew; + socketError(std::move(ew)); } }