Commit 540b1887 authored by Kenton Varda's avatar Kenton Varda

Use UnwindDetector to protect against exceptions in all these RPC object desturctors.

parent 2d1c9a53
...@@ -735,28 +735,30 @@ private: ...@@ -735,28 +735,30 @@ private:
: RpcClient(connectionState), importId(importId) {} : RpcClient(connectionState), importId(importId) {}
~ImportClient() noexcept(false) { ~ImportClient() noexcept(false) {
// Remove self from the import table, if the table is still pointing at us. (It's possible unwindDetector.catchExceptionsIfUnwinding([&]() {
// that another thread attempted to obtain this import just as the destructor started, in // Remove self from the import table, if the table is still pointing at us. (It's possible
// which case that other thread will have constructed a new ImportClient and placed it in // that another thread attempted to obtain this import just as the destructor started, in
// the import table. Therefore, we must actually verify that the import table points at // which case that other thread will have constructed a new ImportClient and placed it in
// this object.) // the import table. Therefore, we must actually verify that the import table points at
KJ_IF_MAYBE(import, connectionState->imports.find(importId)) { // this object.)
KJ_IF_MAYBE(i, import->importClient) { KJ_IF_MAYBE(import, connectionState->imports.find(importId)) {
if (i == this) { KJ_IF_MAYBE(i, import->importClient) {
connectionState->imports.erase(importId); if (i == this) {
connectionState->imports.erase(importId);
}
} }
} }
}
// Send a message releasing our remote references. // Send a message releasing our remote references.
if (remoteRefcount > 0) { if (remoteRefcount > 0) {
auto message = connectionState->connection->newOutgoingMessage( auto message = connectionState->connection->newOutgoingMessage(
messageSizeHint<rpc::Release>()); messageSizeHint<rpc::Release>());
rpc::Release::Builder builder = message->getBody().initAs<rpc::Message>().initRelease(); rpc::Release::Builder builder = message->getBody().initAs<rpc::Message>().initRelease();
builder.setId(importId); builder.setId(importId);
builder.setReferenceCount(remoteRefcount); builder.setReferenceCount(remoteRefcount);
message->send(); message->send();
} }
});
} }
void addRemoteRef() { void addRemoteRef() {
...@@ -794,6 +796,8 @@ private: ...@@ -794,6 +796,8 @@ private:
uint remoteRefcount = 0; uint remoteRefcount = 0;
// Number of times we've received this import from the peer. // Number of times we've received this import from the peer.
kj::UnwindDetector unwindDetector;
}; };
class PipelineClient final: public RpcClient { class PipelineClient final: public RpcClient {
...@@ -1372,13 +1376,15 @@ private: ...@@ -1372,13 +1376,15 @@ private:
CapInjectorImpl(RpcConnectionState& connectionState) CapInjectorImpl(RpcConnectionState& connectionState)
: connectionState(connectionState) {} : connectionState(connectionState) {}
~CapInjectorImpl() noexcept(false) { ~CapInjectorImpl() noexcept(false) {
kj::Vector<kj::Own<ResolutionChain>> thingsToRelease(exports.size()); unwindDetector.catchExceptionsIfUnwinding([&]() {
kj::Vector<kj::Own<ResolutionChain>> thingsToRelease(exports.size());
if (connectionState.networkException == nullptr) { if (connectionState.networkException == nullptr) {
for (auto exportId: exports) { for (auto exportId: exports) {
thingsToRelease.add(connectionState.releaseExport(exportId, 1)); thingsToRelease.add(connectionState.releaseExport(exportId, 1));
}
} }
} });
} }
bool hasCaps() { bool hasCaps() {
...@@ -1451,6 +1457,8 @@ private: ...@@ -1451,6 +1457,8 @@ private:
kj::Vector<ExportId> exports; kj::Vector<ExportId> exports;
// IDs of objects exported during finishDescriptors(). These will need to be released later. // IDs of objects exported during finishDescriptors(). These will need to be released later.
kj::UnwindDetector unwindDetector;
static const void* identity(const rpc::CapDescriptor::Reader& desc) { static const void* identity(const rpc::CapDescriptor::Reader& desc) {
// TODO(cleanup): Don't rely on internal APIs here. // TODO(cleanup): Don't rely on internal APIs here.
return _::PointerHelpers<rpc::CapDescriptor>::getInternalReader(desc).getLocation(); return _::PointerHelpers<rpc::CapDescriptor>::getInternalReader(desc).getLocation();
...@@ -1471,40 +1479,42 @@ private: ...@@ -1471,40 +1479,42 @@ private:
: connectionState(kj::addRef(connectionState)), id(id), fulfiller(kj::mv(fulfiller)) {} : connectionState(kj::addRef(connectionState)), id(id), fulfiller(kj::mv(fulfiller)) {}
~QuestionRef() { ~QuestionRef() {
if (connectionState->networkException != nullptr) { unwindDetector.catchExceptionsIfUnwinding([&]() {
return; if (connectionState->networkException != nullptr) {
} return;
}
// Send the "Finish" message. // Send the "Finish" message.
{ {
uint retainedListSizeHint = resultCaps uint retainedListSizeHint = resultCaps
.map([](kj::Own<CapExtractorImpl>& caps) { return caps->retainedListSizeHint(true); }) .map([](kj::Own<CapExtractorImpl>& caps) { return caps->retainedListSizeHint(true); })
.orDefault(0); .orDefault(0);
auto message = connectionState->connection->newOutgoingMessage( auto message = connectionState->connection->newOutgoingMessage(
messageSizeHint<rpc::Finish>() + retainedListSizeHint); messageSizeHint<rpc::Finish>() + retainedListSizeHint);
auto builder = message->getBody().getAs<rpc::Message>().initFinish(); auto builder = message->getBody().getAs<rpc::Message>().initFinish();
builder.setQuestionId(id); builder.setQuestionId(id);
CapExtractorImpl::FinalizedRetainedCaps retainedCaps; CapExtractorImpl::FinalizedRetainedCaps retainedCaps;
KJ_IF_MAYBE(caps, resultCaps) { KJ_IF_MAYBE(caps, resultCaps) {
retainedCaps = caps->get()->finalizeRetainedCaps( retainedCaps = caps->get()->finalizeRetainedCaps(
Orphanage::getForMessageContaining(builder)); Orphanage::getForMessageContaining(builder));
}
builder.adoptRetainedCaps(kj::mv(retainedCaps.exportList));
message->send();
} }
builder.adoptRetainedCaps(kj::mv(retainedCaps.exportList));
message->send();
}
// Check if the question has returned and, if so, remove it from the table. // Check if the question has returned and, if so, remove it from the table.
// Remove question ID from the table. Must do this *after* sending `Finish` to ensure that // Remove question ID from the table. Must do this *after* sending `Finish` to ensure that
// the ID is not re-allocated before the `Finish` message can be sent. // the ID is not re-allocated before the `Finish` message can be sent.
auto& question = KJ_ASSERT_NONNULL( auto& question = KJ_ASSERT_NONNULL(
connectionState->questions.find(id), "Question ID no longer on table?"); connectionState->questions.find(id), "Question ID no longer on table?");
if (question.paramCaps == nullptr) { if (question.paramCaps == nullptr) {
// Call has already returned, so we can now remove it from the table. // Call has already returned, so we can now remove it from the table.
KJ_ASSERT(connectionState->questions.erase(id)); KJ_ASSERT(connectionState->questions.erase(id));
} else { } else {
question.selfRef = nullptr; question.selfRef = nullptr;
} }
});
} }
inline QuestionId getId() const { return id; } inline QuestionId getId() const { return id; }
...@@ -1533,6 +1543,7 @@ private: ...@@ -1533,6 +1543,7 @@ private:
QuestionId id; QuestionId id;
kj::Own<kj::PromiseFulfiller<kj::Promise<kj::Own<RpcResponse>>>> fulfiller; kj::Own<kj::PromiseFulfiller<kj::Promise<kj::Own<RpcResponse>>>> fulfiller;
kj::Maybe<kj::Own<CapExtractorImpl>> resultCaps; kj::Maybe<kj::Own<CapExtractorImpl>> resultCaps;
kj::UnwindDetector unwindDetector;
}; };
class RpcRequest final: public RequestHook { class RpcRequest final: public RequestHook {
...@@ -2811,18 +2822,20 @@ public: ...@@ -2811,18 +2822,20 @@ public:
} }
~Impl() noexcept(false) { ~Impl() noexcept(false) {
// std::unordered_map doesn't like it when elements' destructors throw, so carefully unwindDetector.catchExceptionsIfUnwinding([&]() {
// disassemble it. // std::unordered_map doesn't like it when elements' destructors throw, so carefully
if (!connections.empty()) { // disassemble it.
kj::Vector<kj::Own<RpcConnectionState>> deleteMe(connections.size()); if (!connections.empty()) {
kj::Exception shutdownException( kj::Vector<kj::Own<RpcConnectionState>> deleteMe(connections.size());
kj::Exception::Nature::LOCAL_BUG, kj::Exception::Durability::PERMANENT, kj::Exception shutdownException(
__FILE__, __LINE__, kj::str("RpcSystem was destroyed.")); kj::Exception::Nature::LOCAL_BUG, kj::Exception::Durability::PERMANENT,
for (auto& entry: connections) { __FILE__, __LINE__, kj::str("RpcSystem was destroyed."));
entry.second->disconnect(kj::cp(shutdownException)); for (auto& entry: connections) {
deleteMe.add(kj::mv(entry.second)); entry.second->disconnect(kj::cp(shutdownException));
deleteMe.add(kj::mv(entry.second));
}
} }
} });
} }
Capability::Client restore(_::StructReader hostId, ObjectPointer::Reader objectId) { Capability::Client restore(_::StructReader hostId, ObjectPointer::Reader objectId) {
...@@ -2850,6 +2863,8 @@ private: ...@@ -2850,6 +2863,8 @@ private:
ConnectionMap; ConnectionMap;
ConnectionMap connections; ConnectionMap connections;
kj::UnwindDetector unwindDetector;
RpcConnectionState& getConnectionState(kj::Own<VatNetworkBase::Connection>&& connection) { RpcConnectionState& getConnectionState(kj::Own<VatNetworkBase::Connection>&& connection) {
auto iter = connections.find(connection); auto iter = connections.find(connection);
if (iter == connections.end()) { if (iter == connections.end()) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment