From c7606670473ae1d07535ec1bf1eb020f3d527539 Mon Sep 17 00:00:00 2001 From: Samuel Warfield Date: Tue, 21 Mar 2023 12:16:24 -0600 Subject: [PATCH] Fix Socket's WorkerInterface Lifetime In long lived actors it is possible that the promise provided by `connect()` can outlive the WorkerInterface that is used to create it's http client. this can lead to unexpected Segfaults when the socket is deconstructed. The object returned by `kj::newHttpClient(HttpService& service)` does not take ownership of the underlying HttpService, whereas asHttpClient() does. This way the WorkerInterface providing the HttpClient will live for the length of the promise. This also done for the equivalent client in the `fetch()` implementation for WebSockets. --- src/workerd/api/sockets.c++ | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/workerd/api/sockets.c++ b/src/workerd/api/sockets.c++ index 3ffe454b2dd..500337653b7 100644 --- a/src/workerd/api/sockets.c++ +++ b/src/workerd/api/sockets.c++ @@ -4,6 +4,7 @@ #include "sockets.h" #include "system-streams.h" +#include namespace workerd::api { @@ -64,12 +65,13 @@ jsg::Ref connectImplNoOutputLock( // Set up the connection. auto headers = kj::heap(ioContext.getHeaderTable()); - auto httpClient = kj::newHttpClient(*client); + auto httpClient = asHttpClient(kj::mv(client)); kj::HttpConnectSettings httpConnectSettings = { .useTls = false }; KJ_IF_MAYBE(opts, options) { httpConnectSettings.useTls = opts->useSecureTransport; } auto request = httpClient->connect(addressStr, *headers, httpConnectSettings); + request.connection = request.connection.attach(kj::mv(httpClient)); auto connDisconnPromise = request.connection->whenWriteDisconnected(); // Initialise the readable/writable streams with the readable/writable sides of an AsyncIoStream.