-
Notifications
You must be signed in to change notification settings - Fork 335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Durable Objects (in-memory only, for testing) #15
Conversation
…n-promise. The next commit will do the same for `makeWorker()`, so that services are constructed entirely synchronously, which is needed to simplify cross-linking service bindings.
Previously, WorkerService's constructor took a list of `Service`s implemeting subrequest channels, which implied that those services had to be constructed before the WorkerService, and so cycles were impossible. We now split into two stages, construction and linking. We construct all services first, then we link them. This also means we no longer have any service types that need to be constructed asynchronously, which improves error handling since all errors will be reported by the time we've fininished constructing+linking.
…ice>. We can make it so all the service objects it returns are long-lived. This fixes a segfault on shutdown because destroying an `Own<T>` that uses `NullDisposer` still requires the object is live if it is polymorphic, in order to `dynamic_cast<void*>` it. We can't easily ensure shutdown order here.
c71ea23
to
d879226
Compare
@@ -378,6 +485,15 @@ void WorkerdApiIsolate::compileGlobals( | |||
value = lock.wrap(context, kj::mv(importedKey)); | |||
} | |||
|
|||
KJ_CASE_ONEOF(ns, Global::EphemeralActorNamespace) { | |||
value = lock.wrap(context, jsg::alloc<api::ColoLocalActorNamespace>(ns.actorChannel)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional that this type is in the repo? I'm fairly surprised to see this type here at all, but especially without a name change to something like EphemeralActorNamespace
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think it'd make sense to rename it. Only people actually looking at the C++ code would see this name, and I'm not too worried about them learning that these are historically colo-local, but a rename would make the code clearer for sure.
This implementation of Durable Objects is not actually... "durable". Storage is kept in-memory (in
ActorCache
) and blown away when the process restarts. But, this is good enough to use for local testing of DO-based applications.Eventually, we'd like to have an actual durable storage back-end in
workerd
. The implementation we use for Cloudflare Workers, though, is deeply coupled to our network details and so wouldn't be very useful to release as-is.