Skip to content

Commit

Permalink
Make server.stop return a Promise that fulfills when all opened conne…
Browse files Browse the repository at this point in the history
…ctions are closed (#14120)
  • Loading branch information
Jarred-Sumner authored Sep 24, 2024
1 parent 0ac2a7d commit 17d719f
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 15 deletions.
2 changes: 1 addition & 1 deletion packages/bun-types/bun.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2660,7 +2660,7 @@ declare module "bun" {
* @param closeActiveConnections Immediately terminate in-flight requests, websockets, and stop accepting new connections.
* @default false
*/
stop(closeActiveConnections?: boolean): void;
stop(closeActiveConnections?: boolean): Promise<void>;

/**
* Update the `fetch` and `error` handlers without restarting the server.
Expand Down
26 changes: 21 additions & 5 deletions src/bun.js/api/server.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6300,6 +6300,8 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp
}

pub fn stopFromJS(this: *ThisServer, abruptly: ?JSValue) JSC.JSValue {
const rc = this.getAllClosedPromise(this.globalThis);

if (this.listener != null) {
const abrupt = brk: {
if (abruptly) |val| {
Expand All @@ -6315,7 +6317,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp
this.stop(abrupt);
}

return .undefined;
return rc;
}

pub fn disposeFromJS(this: *ThisServer) JSC.JSValue {
Expand Down Expand Up @@ -6508,6 +6510,18 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp
return this.activeSocketsCount() > 0;
}

pub fn getAllClosedPromise(this: *ThisServer, globalThis: *JSC.JSGlobalObject) JSC.JSValue {
if (this.listener == null and this.pending_requests == 0) {
return JSC.JSPromise.resolvedPromise(globalThis, .undefined).asValue(globalThis);
}
const prom = &this.all_closed_promise;
if (prom.strong.has()) {
return prom.value();
}
prom.* = JSC.JSPromise.Strong.init(globalThis);
return prom.value();
}

pub fn deinitIfWeCan(this: *ThisServer) void {
httplog("deinitIfWeCan", .{});

Expand All @@ -6522,10 +6536,12 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp

const task = ServerAllConnectionsClosedTask.new(.{
.globalObject = this.globalThis,
.promise = this.all_closed_promise,
// Duplicate the Strong handle so that we can hold two independent strong references to it.
.promise = JSC.JSPromise.Strong{
.strong = JSC.Strong.create(this.all_closed_promise.value(), this.globalThis),
},
.tracker = JSC.AsyncTaskTracker.init(vm),
});
this.all_closed_promise = .{};
event_loop.enqueueTask(JSC.Task.init(task));
}
if (this.pending_requests == 0 and this.listener == null and this.flags.has_js_deinited and !this.hasActiveWebSockets()) {
Expand Down Expand Up @@ -6588,6 +6604,7 @@ pub fn NewServer(comptime NamespaceType: type, comptime ssl_enabled_: bool, comp
httplog("deinit", .{});
this.cached_hostname.deref();
this.cached_protocol.deref();
this.all_closed_promise.deinit();

this.config.deinit();
this.app.destroy();
Expand Down Expand Up @@ -7165,12 +7182,11 @@ pub const ServerAllConnectionsClosedTask = struct {
defer tracker.didDispatch(globalObject);

var promise = this.promise;
defer promise.deinit();
this.destroy();

if (!vm.isShuttingDown()) {
promise.resolve(globalObject, .undefined);
} else {
promise.deinit();
}
}
};
Expand Down
10 changes: 1 addition & 9 deletions src/bun.js/node/node_http_binding.zig
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,7 @@ pub fn getBunServerAllClosedPromise(globalThis: *JSC.JSGlobalObject, callframe:
JSC.API.DebugHTTPSServer,
}) |Server| {
if (value.as(Server)) |server| {
if (server.listener == null and server.pending_requests == 0) {
return JSC.JSPromise.resolvedPromise(globalThis, .undefined).asValue(globalThis);
}
const prom = &server.all_closed_promise;
if (prom.strong.has()) {
return prom.value();
}
prom.* = JSC.JSPromise.Strong.init(globalThis);
return prom.value();
return server.getAllClosedPromise(globalThis);
}
}

Expand Down
72 changes: 72 additions & 0 deletions test/js/bun/http/bun-server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,78 @@ test("Bun should be able to handle utf16 inside Content-Type header #11316", asy
expect(result.headers.get("Content-Type")).toBe("text/html");
});

test("should be able to await server.stop()", async () => {
const { promise, resolve } = Promise.withResolvers();
const ready = Promise.withResolvers();
const received = Promise.withResolvers();
using server = Bun.serve({
port: 0,
// Avoid waiting for DNS resolution in fetch()
hostname: "127.0.0.1",
async fetch(req) {
received.resolve();
await ready.promise;
return new Response("Hello World", {
headers: {
// Prevent Keep-Alive from keeping the connection open
"Connection": "close",
},
});
},
});

// Start the request
const responsePromise = fetch(server.url);
// Wait for the server to receive it.
await received.promise;
// Stop listening for new connections
const stopped = server.stop();
// Continue the request
ready.resolve();
// Wait for the response
await (await responsePromise).text();
// Wait for the server to stop
await stopped;
// Ensure the server is completely stopped
expect(async () => await fetch(server.url)).toThrow();
});

test("should be able to await server.stop(true) with keep alive", async () => {
const { promise, resolve } = Promise.withResolvers();
const ready = Promise.withResolvers();
const received = Promise.withResolvers();
using server = Bun.serve({
port: 0,
// Avoid waiting for DNS resolution in fetch()
hostname: "127.0.0.1",
async fetch(req) {
received.resolve();
await ready.promise;
return new Response("Hello World");
},
});

// Start the request
const responsePromise = fetch(server.url);
// Wait for the server to receive it.
await received.promise;
// Stop listening for new connections
const stopped = server.stop(true);
// Continue the request
ready.resolve();

// Wait for the server to stop
await stopped;

// It should fail before the server responds
expect(async () => {
await (await responsePromise).text();
}).toThrow();

// Ensure the server is completely stopped
expect(async () => await fetch(server.url)).toThrow();
});

test("should be able to async upgrade using custom protocol", async () => {
const { promise, resolve } = Promise.withResolvers<{ code: number; reason: string } | boolean>();
using server = Bun.serve<unknown>({
Expand Down

0 comments on commit 17d719f

Please sign in to comment.