Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[o11y] Fix segfault with tail worker self loop #3439

Merged
merged 1 commit into from
Jan 30, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/workerd/server/server.c++
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,12 @@ class Server::Service {

// Returns true if the service exports the given handler, e.g. `fetch`, `scheduled`, etc.
virtual bool hasHandler(kj::StringPtr handlerName) = 0;

// Return the service itself, or the underlying service if this instance wraps another service as
// with EntrypointService.
virtual Service* service() {
return this;
}
};

Server::~Server() noexcept {
Expand Down Expand Up @@ -1895,6 +1901,7 @@ class Server::WorkerService final: public Service,
kj::Vector<kj::Own<WorkerInterface>> legacyList;
kj::Vector<kj::Own<WorkerInterface>> streamingList;
for (auto& service: channels.tails) {
KJ_ASSERT(service->service() != this, "A worker currently cannot log to itself");
if (service->hasHandler("tailStream"_kj)) {
streamingList.add(service->startRequest({}));
} else if (service->hasHandler("tail") || service->hasHandler("trace")) {
Expand All @@ -1905,12 +1912,14 @@ class Server::WorkerService final: public Service,
streamingTailWorkers = streamingList.releaseAsArray();
} else {
legacyTailWorkers = KJ_MAP(service, channels.tails) -> kj::Own<WorkerInterface> {
KJ_ASSERT(service != this, "A worker currently cannot log to itself");
// Caution here... if the tail worker ends up have a cirular dependency
// on the worker we'll end up with an infinite loop trying to initialize.
// We can test this directly but it's more difficult to test indirect
// loops (dependency of dependency, etc). Here we're just going to keep
// it simple and just check the direct dependency.
// If service refers to an EntrypointService, we need to compare with the underlying
// WorkerService to match this.
KJ_ASSERT(service->service() != this, "A worker currently cannot log to itself");
return service->startRequest({});
};
}
Expand Down Expand Up @@ -2473,6 +2482,11 @@ class Server::WorkerService final: public Service,
return handlers.contains(handlerName);
}

// Return underlying WorkerService.
virtual Service* service() override {
return &worker;
}

private:
WorkerService& worker;
kj::Maybe<kj::StringPtr> entrypoint;
Expand Down