Commit 40d86592 authored by Kenton Varda's avatar Kenton Varda

Lift requirement that releaseParams() be called before tailCall() or…

Lift requirement that releaseParams() be called before tailCall() or allowCancellation() -- this is no longer necessary given the protocol simplification that allowed cap descriptors to be interpreted immediately upon receipt.
parent 110ca1c0
......@@ -572,7 +572,6 @@ public:
auto tailRequest = params.get("callee").as<DynamicCapability>().newRequest("foo");
tailRequest.set("i", params.get("i"));
tailRequest.set("t", "from TestTailCaller");
context.releaseParams();
return context.tailCall(kj::mv(tailRequest));
} else {
KJ_FAIL_ASSERT("Method not implemented", methodName);
......
......@@ -128,7 +128,6 @@ public:
}
ClientHook::VoidPromiseAndPipeline directTailCall(kj::Own<RequestHook>&& request) override {
KJ_REQUIRE(response == nullptr, "Can't call tailCall() after initializing the results struct.");
releaseParams();
auto promise = request->send();
......@@ -144,7 +143,6 @@ public:
return kj::mv(paf.promise);
}
void allowCancellation() override {
KJ_REQUIRE(request == nullptr, "Must call releaseParams() before allowCancellation().");
cancelAllowedFulfiller->fulfill();
}
kj::Own<CallContextHook> addRef() override {
......
......@@ -236,7 +236,6 @@ public:
// results never actually pass through this machine. Even if no such optimization is possible,
// `tailCall()` may allow pipelined calls to be forwarded optimistically to the new call site.
//
// `tailCall()` implies a call to `releaseParams()`, to simplify certain implementations.
// In general, this should be the last thing a method implementation calls, and the promise
// returned from `tailCall()` should then be returned by the method implementation.
......@@ -252,10 +251,6 @@ public:
// executing on a local thread. The method must perform an asynchronous operation or call
// `EventLoop::current().runLater()` to yield control.
//
// Currently, you must call `releaseParams()` before `allowCancellation()`, otherwise the
// latter will throw an exception. This is a limitation of the current RPC implementation, but
// this requirement could be lifted in the future.
//
// Note: You might think that we should offer `onCancel()` and/or `isCanceled()` methods that
// provide notification when the caller cancels the request without forcefully killing off the
// promise chain. Unfortunately, this composes poorly with promise forking: the canceled
......
......@@ -1665,8 +1665,6 @@ private:
ClientHook::VoidPromiseAndPipeline directTailCall(kj::Own<RequestHook>&& request) override {
KJ_REQUIRE(response == nullptr,
"Can't call tailCall() after initializing the results struct.");
KJ_REQUIRE(this->request == nullptr,
"Must call releaseParams() before tailCall().");
if (request->getBrand() == connectionState.get() && !redirectResults) {
// The tail call is headed towards the peer that called us in the first place, so we can
......@@ -1711,16 +1709,6 @@ private:
return kj::mv(paf.promise);
}
void allowCancellation() override {
// TODO(cleanup): We need to drop the request because it is holding on to the resolution
// chain which in turn holds on to the pipeline which holds on to this object thus
// preventing cancellation from working. This is a bit silly because obviously our
// request couldn't contain PromisedAnswers referring to itself, but currently the chain
// is a linear list and we have no way to tell that a reference to the chain taken before
// a call started doesn't really need to hold the call open. To fix this we'd presumably
// need to make the answer table snapshot-able and have CapExtractorImpl take a snapshot
// at creation.
KJ_REQUIRE(request == nullptr, "Must call releaseParams() before allowCancellation().");
bool previouslyRequestedButNotAllowed = cancellationFlags == CANCEL_REQUESTED;
cancellationFlags |= CANCEL_ALLOWED;
......
......@@ -943,7 +943,6 @@ kj::Promise<void> TestTailCallerImpl::foo(FooContext context) {
auto tailRequest = params.getCallee().fooRequest();
tailRequest.setI(params.getI());
tailRequest.setT("from TestTailCaller");
context.releaseParams();
return context.tailCall(kj::mv(tailRequest));
}
......@@ -1015,7 +1014,6 @@ kj::Promise<void> TestMoreStuffImpl::neverReturn(NeverReturnContext context) {
// Also attach `cap` to the result struct to make sure that is released.
context.getResults().setCapCopy(context.getParams().getCap());
context.releaseParams();
context.allowCancellation();
return kj::mv(promise);
}
......@@ -1059,7 +1057,6 @@ kj::Promise<void> TestMoreStuffImpl::echo(EchoContext context) {
kj::Promise<void> TestMoreStuffImpl::expectCancel(ExpectCancelContext context) {
auto cap = context.getParams().getCap();
context.releaseParams();
context.allowCancellation();
return loop(0, cap, context);
}
......
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