Conversation
We will wait for all in-flight requests to complete, without accepting any new requests. Depends on: capnproto/capnproto#1598
| auto [ fatalPromise, fatalFulfiller ] = kj::newPromiseAndFulfiller<void>(); | ||
| this->fatalFulfiller = kj::mv(fatalFulfiller); | ||
|
|
||
| auto forkedDrainWhen = drainWhen |
There was a problem hiding this comment.
Is the behavior correct if drainWhen is rejected for some reason? I guess that would just let the server keep running.
There was a problem hiding this comment.
Hmm, yeah I suppose you could argue that drain() should be called on HttpServers in the failure case too.
However, in practice, if drainWhen fails, then by virtue of it being exclusiveJoin()ed with the listen tasks, those tasks will fail, causing Server::taskFailed() to be invoked, which fulfills the "fatal fulfiller", which in turn causes Server::run() itself to fail with the given error. So the effect is, the drain is skipped and the server shuts down immediately.
I think that works out just fine so I think I'm OK with the code as it is.
| auto promise = kj::evalNow([&]() { | ||
| return conn->listedHttp.httpServer.listenHttp(kj::mv(stream.stream)) | ||
| .attach(kj::mv(conn)) | ||
| .attach(kj::addRef(*this)); // two attach()es because `this` must outlive `conn` |
There was a problem hiding this comment.
I thought .attach(a, b) is always equivalent to .attach(a).attach(b). But, from looking at the code it looks like that's only true for kj::Own<T>::attach(), and with kj::Promise<T>::attach() the destruction order is reversed?
There was a problem hiding this comment.
Yes, unfortunately, I didn't think hard enough when creating kj::Promise::attach() and it's probably too late to safely reverse the ordering. To avoid any confusion I use multiple attaches when the ordering matters.
There was a problem hiding this comment.
Ah, I see. I suppose we could deprecate both attach()es in favor of a fixed attach2() for a couple capnproto releases, then deprecate-and-rename back to attach().
|
(Waiting for capnproto/capnproto#1598 to be approved before merging this.) |
|
I approved the capnproto PR. |
`kill()` uses `SIGTERM` by default. In `workerd`, this waits for HTTP connections to close before exiting. Notably, Chrome sometimes keeps connections open for about 10s, blocking exit. We'd like `dispose()`/ `setOptions()` to immediately terminate the existing process. Therefore, use `SIGINT` which force closes all connections. Ref: cloudflare/workerd#244
`kill()` uses `SIGTERM` by default. In `workerd`, this waits for HTTP connections to close before exiting. Notably, Chrome sometimes keeps connections open for about 10s, blocking exit. We'd like `dispose()`/ `setOptions()` to immediately terminate the existing process. Therefore, use `SIGINT` which force closes all connections. Ref: cloudflare/workerd#244
`kill()` uses `SIGTERM` by default. In `workerd`, this waits for HTTP connections to close before exiting. Notably, Chrome sometimes keeps connections open for about 10s, blocking exit. We'd like `dispose()`/ `setOptions()` to immediately terminate the existing process. Therefore, use `SIGINT` which force closes all connections. Ref: cloudflare/workerd#244
`kill()` uses `SIGTERM` by default. In `workerd`, this waits for HTTP connections to close before exiting. Notably, Chrome sometimes keeps connections open for about 10s, blocking exit. We'd like `dispose()`/ `setOptions()` to immediately terminate the existing process. Therefore, use `SIGINT` which force closes all connections. Ref: cloudflare/workerd#244
`kill()` uses `SIGTERM` by default. In `workerd`, this waits for HTTP connections to close before exiting. Notably, Chrome sometimes keeps connections open for about 10s, blocking exit. We'd like `dispose()`/ `setOptions()` to immediately terminate the existing process. Therefore, use `SIGINT` which force closes all connections. Ref: cloudflare/workerd#244
3 participants
We will wait for all in-flight requests to complete, without accepting any new requests.
Depends on: capnproto/capnproto#1598
Addresses part of #101.