facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-09 16:31:02 +03:00
Code Issues Projects Releases Wiki Activity

nfs: fix RpcServer lifetime

Browse Source 
Summary:
Take 2 of fixing the lifetime issues of the RpcServer, this time, the
RpcAcceptCallback needs to live for longer than RpcServer itself. This is due
to the acceptStopped callback being called after RpcServer is being destroyed.
Since the RpcServer needs to be destroyed in the EventBase thread, the
acceptStopped callback is delayed, causing it to be invoked after the RpcServer
has freed its memory.

To solve this, we simply need to make the RpcAcceptCallback a delayed
destructor, and hold a reference to itself that goes away once the
acceptStopped callback is called.

Reviewed By: kmancini

Differential Revision: D26556644

fbshipit-source-id: 32cd80f1664649c1ad96f88c4eec6dd2ed6e8c12
This commit is contained in:
Xavier Deguillard 2021-02-22 22:38:29 -08:00 committed by Facebook GitHub Bot
parent 52f2984156
commit 206d0ced85
2 changed files with 49 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
eden/fs/nfs/rpc/Server.cpp
View File

@ -181,7 +181,7 @@ class RpcTcpHandler : public folly::DelayedDestruction {
}
std::shared_ptr<RpcServerProcessor> proc_;
std::unique_ptr<AsyncSocket, ReleasableDestructor> sock_;
AsyncSocket::UniquePtr sock_;
std::unique_ptr<Reader> reader_;
Writer writer_;
folly::IOBufQueue readBuf_;
@ -268,6 +268,17 @@ void RpcServer::RpcAcceptCallback::connectionAccepted(
handler->setup();
}
void RpcServer::RpcAcceptCallback::acceptError(
const std::exception& ex) noexcept {
XLOG(ERR) << "acceptError: " << folly::exceptionStr(ex);
}
void RpcServer::RpcAcceptCallback::acceptStopped() noexcept {
// We won't ever be accepting any connection, it is now safe to delete
// ourself, release the guard.
guard_.reset();
}
auth_stat RpcServerProcessor::checkAuthentication(
const call_body& /*call_body*/) {
// Completely ignore authentication.
@ -289,13 +300,13 @@ RpcServer::RpcServer(
std::shared_ptr<RpcServerProcessor> proc,
folly::EventBase* evb)
: evb_(evb),
acceptCb_(proc, evb_),
serverSocket_(AsyncServerSocket::newSocket(evb_)) {
acceptCb_(new RpcServer::RpcAcceptCallback(proc, evb_)),
serverSocket_(new AsyncServerSocket(evb_)) {
// Ask kernel to assign us a port on the loopback interface
serverSocket_->bind(SocketAddress("127.0.0.1", 0));
serverSocket_->listen(1024);
serverSocket_->addAcceptCallback(&acceptCb_, evb_);
serverSocket_->addAcceptCallback(acceptCb_.get(), evb_);
serverSocket_->startAccepting();
}

57
eden/fs/nfs/rpc/Server.h
View File

@ -11,7 +11,6 @@
#include <folly/SocketAddress.h>
#include <folly/io/async/AsyncServerSocket.h>
#include <folly/logging/xlog.h>
#include <folly/net/NetworkSocket.h>
#include "eden/fs/nfs/portmap/PortmapClient.h"
#include "eden/fs/nfs/rpc/Rpc.h"
@ -32,26 +31,6 @@ class RpcServerProcessor {
};
class RpcServer {
class RpcAcceptCallback : public folly::AsyncServerSocket::AcceptCallback {
public:
explicit RpcAcceptCallback(
std::shared_ptr<RpcServerProcessor> proc,
folly::EventBase* evb)
: evb_(evb), proc_(proc) {}
void connectionAccepted(
folly::NetworkSocket fd,
const folly::SocketAddress& clientAddr) noexcept override;
void acceptError(const std::exception& ex) noexcept override {
XLOG(ERR) << "acceptError: " << folly::exceptionStr(ex);
}
private:
folly::EventBase* evb_;
std::shared_ptr<RpcServerProcessor> proc_;
};
public:
RpcServer(std::shared_ptr<RpcServerProcessor> proc, folly::EventBase* evb);
~RpcServer();
@ -71,9 +50,41 @@ class RpcServer {
uint16_t getPort() const;
private:
class RpcAcceptCallback : public folly::AsyncServerSocket::AcceptCallback,
public folly::DelayedDestruction {
public:
using UniquePtr = std::
unique_ptr<RpcAcceptCallback, folly::DelayedDestruction::Destructor>;
explicit RpcAcceptCallback(
std::shared_ptr<RpcServerProcessor> proc,
folly::EventBase* evb)
: evb_(evb), proc_(proc), guard_(this) {}
private:
void connectionAccepted(
folly::NetworkSocket fd,
const folly::SocketAddress& clientAddr) noexcept override;
void acceptError(const std::exception& ex) noexcept override;
void acceptStopped() noexcept override;
~RpcAcceptCallback() override = default;
folly::EventBase* evb_;
std::shared_ptr<RpcServerProcessor> proc_;
/**
* Hold a guard to ourself to avoid being deleted until the callback is
* removed from the AsyncServerSocket.
*/
std::optional<folly::DelayedDestruction::DestructorGuard> guard_;
};
folly::EventBase* evb_;
RpcAcceptCallback acceptCb_;
std::shared_ptr<folly::AsyncServerSocket> serverSocket_;
RpcAcceptCallback::UniquePtr acceptCb_;
folly::AsyncServerSocket::UniquePtr serverSocket_;
PortmapClient portMap_;
std::vector<PortmapMapping> mappedPorts_;
std::shared_ptr<RpcServerProcessor> proc_;