Skip to content

Commit

Permalink
Merge pull request #1462 from cloudflare/jsnell/rename-apiisolate-to-…
Browse files Browse the repository at this point in the history 
…just-api
  • Loading branch information
@jasnell
jasnell authored Dec 5, 2023
2 parents a133abb + 842b33c commit e883178
Show file tree
Hide file tree
Showing 11 changed files with 97 additions and 91 deletions.
4 changes: 2 additions & 2 deletions src/workerd/api/crypto.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ static kj::Maybe<const CryptoAlgorithm&> lookupAlgorithm(kj::StringPtr name) {

auto iter = ALGORITHMS.find(CryptoAlgorithm {name});
if (iter == ALGORITHMS.end()) {
// No such built-in algorithm, so fall back to checking if the ApiIsolate has a custom
// No such built-in algorithm, so fall back to checking if the Api has a custom
// algorithm registered.
return Worker::ApiIsolate::current().getCryptoAlgorithm(name);
return Worker::Api::current().getCryptoAlgorithm(name);
} else {
return *iter;
}
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/queue.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ kj::Promise<WorkerInterface::CustomEvent::Result> QueueCustomEventImpl::run(
(Worker::Lock& lock) mutable {
jsg::AsyncContextFrame::StorageScope traceScope = context.makeAsyncTraceScope(lock);

auto& typeHandler = lock.getWorker().getIsolate().getApiIsolate().getQueueTypeHandler(lock);
auto& typeHandler = lock.getWorker().getIsolate().getApi().getQueueTypeHandler(lock);
queueEvent->event = startQueueEvent(lock.getGlobalScope(), kj::mv(params), context.addObject(result), lock,
lock.getExportedHandler(entrypointName, context.getActor()), typeHandler);
}));
Expand Down
6 changes: 3 additions & 3 deletions src/workerd/io/features.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ namespace workerd {
CompatibilityFlags::Reader FeatureFlags::get(jsg::Lock&) {
// Note that the jsg::Lock& argument here is not actually used. We require
// that a jsg::Lock reference is passed in as proof that current() is called
// from within a valid isolate lock so that the Worker::ApiIsolate::current()
// from within a valid isolate lock so that the Worker::Api::current()
// call below will work as expected.
// TODO(later): Use of Worker::ApiIsolate::current() here implies that there
// TODO(later): Use of Worker::Api::current() here implies that there
// is only one set of compatibility flags relevant at a time within each thread
// context. For now that holds true. Later it is possible that may not be the
// case which will require us to further adapt this model.
return Worker::ApiIsolate::current().getFeatureFlags();
return Worker::Api::current().getFeatureFlags();
}

} // namespace workerd
2 changes: 1 addition & 1 deletion src/workerd/io/worker-entrypoint.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ kj::Promise<WorkerInterface::CustomEvent::Result>
#ifdef KJ_DEBUG
void requestGc(const Worker& worker) {
jsg::V8StackScope stackScope;
auto lock = worker.getIsolate().getApiIsolate().lock(stackScope);
auto lock = worker.getIsolate().getApi().lock(stackScope);
lock->requestGcForTesting();
}

Expand Down
56 changes: 28 additions & 28 deletions src/workerd/io/worker.c++
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ void addExceptionToTrace(jsg::Lock& js,
WorkerTracer& tracer,
UncaughtExceptionSource source,
const jsg::JsValue& exception,
const jsg::TypeHandler<Worker::ApiIsolate::ErrorInterface>&
const jsg::TypeHandler<Worker::Api::ErrorInterface>&
errorTypeHandler) {
if (source == UncaughtExceptionSource::INTERNAL ||
source == UncaughtExceptionSource::INTERNAL_ASYNC) {
Expand Down Expand Up @@ -527,11 +527,11 @@ private:
// Defined later in this file.
void setWebAssemblyModuleHasInstance(jsg::Lock& lock, v8::Local<v8::Context> context);

static thread_local const Worker::ApiIsolate* currentApiIsolate = nullptr;
static thread_local const Worker::Api* currentApi = nullptr;

const Worker::ApiIsolate& Worker::ApiIsolate::current() {
KJ_REQUIRE(currentApiIsolate != nullptr, "not running JavaScript");
return *currentApiIsolate;
const Worker::Api& Worker::Api::current() {
KJ_REQUIRE(currentApi != nullptr, "not running JavaScript");
return *currentApi;
}

struct Worker::Impl {
Expand Down Expand Up @@ -604,10 +604,10 @@ struct Worker::Isolate::Impl {
KJ_UNREACHABLE;
}()),
progressCounter(impl.lockSuccessCount),
oldCurrentApiIsolate(currentApiIsolate),
oldCurrentApi(currentApi),
limitEnforcer(isolate.getLimitEnforcer()),
consoleMode(isolate.consoleMode),
lock(isolate.apiIsolate->lock(stackScope)) {
lock(isolate.api->lock(stackScope)) {
WarnAboutIsolateLockScope::maybeWarn();

// Increment the success count to expose forward progress to all threads.
Expand All @@ -628,10 +628,10 @@ struct Worker::Isolate::Impl {
workerImpl = nullptr;
}

currentApiIsolate = isolate.apiIsolate.get();
currentApi = isolate.api.get();
}
~Lock() noexcept(false) {
currentApiIsolate = oldCurrentApiIsolate;
currentApi = oldCurrentApi;

#ifdef KJ_DEBUG
// We lack a KJ_DASSERT_NONNULL because it would have to look a lot like KJ_IF_SOME, thus
Expand Down Expand Up @@ -712,7 +712,7 @@ struct Worker::Isolate::Impl {
IsolateObserver::LockRecord metrics;
ThreadProgressCounter progressCounter;
bool shouldReportIsolateMetrics = false;
const ApiIsolate* oldCurrentApiIsolate;
const Api* oldCurrentApi;

const IsolateLimitEnforcer& limitEnforcer; // only so we can call getIsolateStats()

Expand Down Expand Up @@ -745,13 +745,13 @@ struct Worker::Isolate::Impl {
// because our GlobalScope object needs to have a function called on it, and any attached
// inspector needs to be notified. JSG doesn't know about these things.

Impl(const ApiIsolate& apiIsolate, IsolateObserver& metrics,
Impl(const Api& api, IsolateObserver& metrics,
IsolateLimitEnforcer& limitEnforcer, InspectorPolicy inspectorPolicy)
: metrics(metrics),
inspectorPolicy(inspectorPolicy),
actorCacheLru(limitEnforcer.getActorCacheLruOptions()) {
jsg::V8StackScope stackScope;
auto lock = apiIsolate.lock(stackScope);
auto lock = api.lock(stackScope);
limitEnforcer.customizeIsolate(lock->v8Isolate);

if (inspectorPolicy != InspectorPolicy::DISALLOW) {
Expand Down Expand Up @@ -1060,26 +1060,26 @@ const HeapSnapshotDeleter HeapSnapshotDeleter::INSTANCE;

} // namespace

Worker::Isolate::Isolate(kj::Own<ApiIsolate> apiIsolateParam,
Worker::Isolate::Isolate(kj::Own<Api> apiParam,
kj::Own<IsolateObserver>&& metricsParam,
kj::StringPtr id,
kj::Own<IsolateLimitEnforcer> limitEnforcerParam,
InspectorPolicy inspectorPolicy,
ConsoleMode consoleMode)
: id(kj::str(id)),
limitEnforcer(kj::mv(limitEnforcerParam)),
apiIsolate(kj::mv(apiIsolateParam)),
api(kj::mv(apiParam)),
consoleMode(consoleMode),
featureFlagsForFl(makeCompatJson(decompileCompatibilityFlagsForFl(apiIsolate->getFeatureFlags()))),
featureFlagsForFl(makeCompatJson(decompileCompatibilityFlagsForFl(api->getFeatureFlags()))),
metrics(kj::mv(metricsParam)),
impl(kj::heap<Impl>(*apiIsolate, *metrics, *limitEnforcer, inspectorPolicy)),
impl(kj::heap<Impl>(*api, *metrics, *limitEnforcer, inspectorPolicy)),
weakIsolateRef(WeakIsolateRef::wrap(this)),
traceAsyncContextKey(kj::refcounted<jsg::AsyncContextFrame::StorageKey>()) {
metrics->created();
// We just created our isolate, so we don't need to use Isolate::Impl::Lock (nor an async lock).
jsg::V8StackScope stackScope;
auto lock = apiIsolate->lock(stackScope);
auto features = apiIsolate->getFeatureFlags();
auto lock = api->lock(stackScope);
auto features = api->getFeatureFlags();

lock->setCaptureThrowsAsRejections(features.getCaptureThrowsAsRejections());
lock->setCommonJsExportDefault(features.getExportCommonJsDefaultNamespace());
Expand Down Expand Up @@ -1217,7 +1217,7 @@ Worker::Script::Script(kj::Own<const Isolate> isolateParam, kj::StringPtr id,
v8::Local<v8::Context> context;
if (modular) {
// Modules can't be compiled for multiple contexts. We need to create the real context now.
auto& mContext = impl->moduleContext.emplace(isolate->apiIsolate->newContext(lock));
auto& mContext = impl->moduleContext.emplace(isolate->getApi().newContext(lock));
mContext->enableWarningOnSpecialEvents();
context = mContext.getHandle(lock);
recordedLock.setupContext(context);
Expand Down Expand Up @@ -1272,7 +1272,7 @@ Worker::Script::Script(kj::Own<const Isolate> isolateParam, kj::StringPtr id,
try {
KJ_SWITCH_ONEOF(source) {
KJ_CASE_ONEOF(script, ScriptSource) {
impl->globals = script.compileGlobals(lock, *isolate->apiIsolate, isolate->impl->metrics);
impl->globals = script.compileGlobals(lock, isolate->getApi(), isolate->impl->metrics);

{
// It's unclear to me if CompileUnboundScript() can get trapped in any infinite loops or
Expand All @@ -1290,7 +1290,7 @@ Worker::Script::Script(kj::Own<const Isolate> isolateParam, kj::StringPtr id,
auto limitScope = isolate->getLimitEnforcer().enterStartupJs(lock, maybeLimitError);
auto& modules = KJ_ASSERT_NONNULL(impl->moduleContext)->getModuleRegistry();
impl->configureDynamicImports(lock, modules);
modulesSource.compileModules(lock, *isolate->apiIsolate);
modulesSource.compileModules(lock, isolate->getApi());
impl->unboundScriptOrMainModule = kj::Path::parse(modulesSource.mainModule);
break;
}
Expand Down Expand Up @@ -1385,7 +1385,7 @@ void setWebAssemblyModuleHasInstance(jsg::Lock& lock, v8::Local<v8::Context> con
Worker::Worker(kj::Own<const Script> scriptParam,
kj::Own<WorkerObserver> metricsParam,
kj::FunctionParam<void(
jsg::Lock& lock, const ApiIsolate& apiIsolate,
jsg::Lock& lock, const Api& api,
v8::Local<v8::Object> target)> compileBindings,
IsolateObserver::StartType startType,
SpanParent parentSpan, LockType lockType,
Expand Down Expand Up @@ -1433,7 +1433,7 @@ Worker::Worker(kj::Own<const Script> scriptParam,
currentSpan.setTag("module_context"_kjc, true);
} else {
// Create a new context.
jsContext = &this->impl->context.emplace(script->isolate->apiIsolate->newContext(lock));
jsContext = &this->impl->context.emplace(script->isolate->getApi().newContext(lock));
}

v8::Local<v8::Context> context = KJ_REQUIRE_NONNULL(jsContext).getHandle(lock);
Expand Down Expand Up @@ -1473,7 +1473,7 @@ Worker::Worker(kj::Own<const Script> scriptParam,
lock.v8Set(bindingsScope, global.name, global.value);
}

compileBindings(lock, *script->isolate->apiIsolate, bindingsScope);
compileBindings(lock, script->isolate->getApi(), bindingsScope);

// Execute script.
currentSpan = maybeMakeSpan("lw:top_level_execution"_kjc);
Expand Down Expand Up @@ -1516,7 +1516,7 @@ Worker::Worker(kj::Own<const Script> scriptParam,

impl->env = lock.v8Ref(bindingsScope.As<v8::Value>());

auto handlers = script->isolate->apiIsolate->unwrapExports(lock, ns);
auto handlers = script->isolate->getApi().unwrapExports(lock, ns);

for (auto& handler: handlers.fields) {
KJ_SWITCH_ONEOF(handler.value) {
Expand Down Expand Up @@ -1857,7 +1857,7 @@ void Worker::Lock::logUncaughtException(UncaughtExceptionSource source,
js.withinHandleScope([&] {
auto contextScope = js.enterContextScope(getContext());
addExceptionToTrace(impl->inner, ioContext, tracer, source, exception,
worker.getIsolate().apiIsolate->getErrorInterfaceTypeHandler(*this));
worker.getIsolate().getApi().getErrorInterfaceTypeHandler(*this));
});
}
}
Expand Down Expand Up @@ -3021,7 +3021,7 @@ void Worker::Actor::ensureConstructed(IoContext& context) {

kj::Maybe<jsg::Ref<api::DurableObjectStorage>> storage;
KJ_IF_SOME(c, impl->actorCache) {
storage = impl->makeStorage(lock, *worker->getIsolate().apiIsolate, *c);
storage = impl->makeStorage(lock, worker->getIsolate().getApi(), *c);
}
auto handler = cls(lock,
jsg::alloc<api::DurableObjectState>(cloneId(), kj::mv(storage)),
Expand Down Expand Up @@ -3162,7 +3162,7 @@ kj::Own<Worker::Actor::Loopback> Worker::Actor::getLoopback() {
kj::Maybe<jsg::Ref<api::DurableObjectStorage>>
Worker::Actor::makeStorageForSwSyntax(Worker::Lock& lock) {
return impl->actorCache.map([&](kj::Own<ActorCacheInterface>& cache) {
return impl->makeStorage(lock, *worker->getIsolate().apiIsolate, *cache);
return impl->makeStorage(lock, worker->getIsolate().getApi(), *cache);
});
}

Expand Down
28 changes: 15 additions & 13 deletions src/workerd/io/worker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ class Worker: public kj::AtomicRefcounted {
public:
class Script;
class Isolate;
class ApiIsolate;
class Api;
using ApiIsolate [[deprecated("Use Workerd::Api")]] = Api;

class ValidationErrorReporter {
public:
Expand All @@ -92,7 +93,7 @@ class Worker: public kj::AtomicRefcounted {
explicit Worker(kj::Own<const Script> script,
kj::Own<WorkerObserver> metrics,
kj::FunctionParam<void(
jsg::Lock& lock, const ApiIsolate& apiIsolate,
jsg::Lock& lock, const Api& api,
v8::Local<v8::Object> target)> compileBindings,
IsolateObserver::StartType startType,
SpanParent parentSpan, LockType lockType,
Expand Down Expand Up @@ -197,7 +198,7 @@ class Worker::Script: public kj::AtomicRefcounted {
kj::StringPtr mainScriptName;

// Callback which will compile the script-level globals, returning a list of them.
kj::Function<kj::Array<CompiledGlobal>(jsg::Lock& lock, const ApiIsolate& apiIsolate, const jsg::CompilationObserver& observer)>
kj::Function<kj::Array<CompiledGlobal>(jsg::Lock& lock, const Api& api, const jsg::CompilationObserver& observer)>
compileGlobals;
};
struct ModulesSource {
Expand All @@ -206,7 +207,7 @@ class Worker::Script: public kj::AtomicRefcounted {
kj::StringPtr mainModule;

// Callback which will construct the module registry and load all the modules into it.
kj::Function<void(jsg::Lock& lock, const ApiIsolate& apiIsolate)> compileModules;
kj::Function<void(jsg::Lock& lock, const Api& api)> compileModules;
};
using Source = kj::OneOf<ScriptSource, ModulesSource>;

Expand Down Expand Up @@ -252,7 +253,7 @@ class Worker::Isolate: public kj::AtomicRefcounted {
// Usually it matches the script ID. An exception is preview isolates: there, each preview
// session has one isolate which may load many iterations of the script (this allows the
// inspector session to stay open across them).
explicit Isolate(kj::Own<ApiIsolate> apiIsolate,
explicit Isolate(kj::Own<Api> api,
kj::Own<IsolateObserver>&& metrics,
kj::StringPtr id,
kj::Own<IsolateLimitEnforcer> limitEnforcer,
Expand All @@ -273,7 +274,9 @@ class Worker::Isolate: public kj::AtomicRefcounted {
kj::Maybe<ValidationErrorReporter&> errorReporter = kj::none) const;

const IsolateLimitEnforcer& getLimitEnforcer() const { return *limitEnforcer; }
const ApiIsolate& getApiIsolate() const { return *apiIsolate; }

const Api& getApi() const { return *api; }
[[deprecated("use getApi()")]] const Api& getApiIsolate() const { return *api; }

// Returns the number of threads currently blocked trying to lock this isolate's mutex (using
// takeAsyncLock()).
Expand Down Expand Up @@ -345,7 +348,7 @@ class Worker::Isolate: public kj::AtomicRefcounted {

kj::String id;
kj::Own<IsolateLimitEnforcer> limitEnforcer;
kj::Own<ApiIsolate> apiIsolate;
kj::Own<Api> api;
ConsoleMode consoleMode;

// If non-null, a serialized JSON object with a single "flags" property, which is a list of
Expand Down Expand Up @@ -406,11 +409,10 @@ class Worker::Isolate: public kj::AtomicRefcounted {
//
// In contrast, the rest of the classes in `worker.h` are concerned more with lifecycle
// management.
class Worker::ApiIsolate {
// TODO(cleanup): Find a better name for this, we have too many things called "isolate".
class Worker::Api {
public:
// Get the current `ApiIsolate` or throw if we're not currently executing JavaScript.
static const ApiIsolate& current();
// Get the current `Api` or throw if we're not currently executing JavaScript.
static const Api& current();
// TODO(cleanup): This is a hack thrown in quickly because IoContext::current() doesn't work in
// the global scope (when no request is running). We need a better design here.

Expand All @@ -421,7 +423,7 @@ class Worker::ApiIsolate {
// to the event loop.

// Get the FeatureFlags this isolate is configured with. Returns a Reader that is owned by the
// ApiIsolate.
// Api.
virtual CompatibilityFlags::Reader getFeatureFlags() const = 0;

// Create the context (global scope) object.
Expand Down Expand Up @@ -596,7 +598,7 @@ class Worker::Actor final: public kj::Refcounted {
// Callback which constructs the `DurableObjectStorage` instance for an actor. This can be used
// to customize the JavaScript API.
using MakeStorageFunc = kj::Function<jsg::Ref<api::DurableObjectStorage>(
jsg::Lock& js, const ApiIsolate& apiIsolate, ActorCacheInterface& actorCache)>;
jsg::Lock& js, const Api& api, ActorCacheInterface& actorCache)>;
// TODO(cleanup): Can we refactor the (internal-codebase) user of this so that it doesn't need
// to customize the JS API but only the underlying ActorCacheInterface?

Expand Down
Loading

0 comments on commit e883178

Please sign in to comment.