Skip to content

Commit 25b6b5f

Browse files
committed
Fixes "promise will never complete" when exceeding memory.
1 parent 842d84c commit 25b6b5f

File tree

7 files changed

+57
-5
lines changed

7 files changed

+57
-5
lines changed

src/workerd/io/BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ wd_cc_library(
125125
"//src/workerd/jsg",
126126
"//src/workerd/util:autogate",
127127
"//src/workerd/util:completion-membrane",
128+
"//src/workerd/util:exception",
128129
"//src/workerd/util:sqlite",
129130
"//src/workerd/util:thread-scopes",
130131
"//src/workerd/util:uuid",

src/workerd/io/io-context.h

+26-5
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <workerd/io/trace.h>
1717
#include <workerd/jsg/async-context.h>
1818
#include <workerd/jsg/jsg.h>
19+
#include <workerd/util/exception.h>
1920
#include <workerd/util/uncaught-exception-source.h>
2021
#include <workerd/util/weak-refs.h>
2122

@@ -1257,23 +1258,42 @@ kj::_::ReducePromises<RemoveIoOwn<T>> IoContext::awaitJs(jsg::Lock& js, jsg::Pro
12571258
auto paf = kj::newPromiseAndFulfiller<RemoveIoOwn<T>>();
12581259
struct RefcountedFulfiller: public Finalizeable, public kj::Refcounted {
12591260
kj::Own<kj::PromiseFulfiller<RemoveIoOwn<T>>> fulfiller;
1261+
kj::Own<const AtomicWeakRef<Worker::Isolate>> maybeIsolate;
12601262
bool isDone = false;
12611263

1262-
RefcountedFulfiller(kj::Own<kj::PromiseFulfiller<RemoveIoOwn<T>>> fulfiller)
1263-
: fulfiller(kj::mv(fulfiller)) {}
1264+
RefcountedFulfiller(kj::Own<const AtomicWeakRef<Worker::Isolate>> maybeIsolate,
1265+
kj::Own<kj::PromiseFulfiller<RemoveIoOwn<T>>> fulfiller)
1266+
: fulfiller(kj::mv(fulfiller)),
1267+
maybeIsolate(kj::mv(maybeIsolate)) {}
12641268

12651269
~RefcountedFulfiller() noexcept(false) {
12661270
if (!isDone) {
1271+
reject();
1272+
}
1273+
}
1274+
1275+
private:
1276+
void reject() {
1277+
// We use a weak isolate reference here in case the isolate gets dropped before this code
1278+
// is executed. In that case we default to `false` as we cannot access the original isolate.
1279+
auto hasExcessivelyExceededHeapLimit = maybeIsolate->tryAddStrongRef()
1280+
.map([](kj::Own<const Worker::Isolate> isolate) {
1281+
return isolate->getLimitEnforcer().hasExcessivelyExceededHeapLimit();
1282+
}).orDefault(false);
1283+
if (hasExcessivelyExceededHeapLimit) {
1284+
auto e = JSG_KJ_EXCEPTION(OVERLOADED, Error, "Worker has exceeded memory limit.");
1285+
e.setDetail(MEMORY_LIMIT_DETAIL_ID, kj::heapArray<kj::byte>(0));
1286+
fulfiller->reject(kj::mv(e));
1287+
} else {
12671288
// The JavaScript resolver was garbage collected, i.e. JavaScript will never resolve
12681289
// this promise.
12691290
fulfiller->reject(JSG_KJ_EXCEPTION(FAILED, Error, "Promise will never complete."));
12701291
}
12711292
}
12721293

1273-
private:
12741294
kj::Maybe<kj::StringPtr> finalize() override {
12751295
if (!isDone) {
1276-
fulfiller->reject(JSG_KJ_EXCEPTION(FAILED, Error, "Promise will never complete."));
1296+
reject();
12771297
isDone = true;
12781298
return "A hanging Promise was canceled. This happens when the worker runtime is waiting "
12791299
"for a Promise from JavaScript to resolve, but has detected that the Promise "
@@ -1284,7 +1304,8 @@ kj::_::ReducePromises<RemoveIoOwn<T>> IoContext::awaitJs(jsg::Lock& js, jsg::Pro
12841304
}
12851305
}
12861306
};
1287-
auto fulfiller = kj::refcounted<RefcountedFulfiller>(kj::mv(paf.fulfiller));
1307+
auto& isolate = Worker::Isolate::from(js);
1308+
auto fulfiller = kj::refcounted<RefcountedFulfiller>(isolate.getWeakRef(), kj::mv(paf.fulfiller));
12881309

12891310
auto errorHandler = [fulfiller = addObject(kj::addRef(*fulfiller))](
12901311
jsg::Lock& js, jsg::Value jsExceptionRef) mutable {

src/workerd/io/limit-enforcer.h

+2
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ class IsolateLimitEnforcer: public kj::Refcounted {
8383
virtual size_t getBlobSizeLimit() const {
8484
return 128 * 1024 * 1024; // 128 MB
8585
}
86+
87+
virtual bool hasExcessivelyExceededHeapLimit() const = 0;
8688
};
8789

8890
// Abstract interface that enforces resource limits on a IoContext.

src/workerd/server/server.c++

+4
Original file line numberDiff line numberDiff line change
@@ -3199,6 +3199,10 @@ kj::Own<Server::Service> Server::makeWorker(kj::StringPtr name,
31993199
// No limit on the number of iterations in workerd
32003200
return kj::none;
32013201
}
3202+
3203+
bool hasExcessivelyExceededHeapLimit() const override {
3204+
return false;
3205+
}
32023206
};
32033207

32043208
auto jsgobserver = kj::atomicRefcounted<JsgIsolateObserver>();

src/workerd/tests/test-fixture.c++

+3
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,9 @@ struct MockIsolateLimitEnforcer final: public IsolateLimitEnforcer {
212212
kj::Maybe<size_t> checkPbkdfIterations(jsg::Lock& lock, size_t iterations) const override {
213213
return kj::none;
214214
}
215+
bool hasExcessivelyExceededHeapLimit() const override {
216+
return false;
217+
}
215218
};
216219

217220
struct MockErrorReporter final: public Worker::ValidationErrorReporter {

src/workerd/util/BUILD.bazel

+7
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,13 @@ wd_cc_library(
210210
deps = ["@capnp-cpp//src/capnp"],
211211
)
212212

213+
wd_cc_library(
214+
name = "exception",
215+
hdrs = ["exception.h"],
216+
visibility = ["//visibility:public"],
217+
deps = ["@capnp-cpp//src/kj"],
218+
)
219+
213220
exports_files(["autogate.h"])
214221

215222
[

src/workerd/util/exception.h

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright (c) 2017-2022 Cloudflare, Inc.
2+
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
3+
// https://opensource.org/licenses/Apache-2.0
4+
5+
#pragma once
6+
7+
#include <kj/exception.h>
8+
9+
namespace workerd {
10+
11+
// If an exception is thrown for exceeding memory limits, it will contain this detail.
12+
constexpr kj::Exception::DetailTypeId MEMORY_LIMIT_DETAIL_ID = 0xbaf76dd7ce5bd8cfull;
13+
14+
} // namespace workerd

0 commit comments

Comments
 (0)