Commit 480e1f48 authored by Kenton Varda's avatar Kenton Varda

Address comments from @harrishancock

parent f7bbbf4e
...@@ -166,8 +166,8 @@ protected: ...@@ -166,8 +166,8 @@ protected:
void init(Event* newEvent); void init(Event* newEvent);
void arm(); void arm();
// Arms the event if init() has already been called and makes future calls to init() return // Arms the event if init() has already been called and makes future calls to init()
// true. // automatically arm the event.
private: private:
Event* event = nullptr; Event* event = nullptr;
......
...@@ -243,7 +243,7 @@ public: ...@@ -243,7 +243,7 @@ public:
// //
// Generally, poll() is most useful in tests. Often, you may want to verify that a promise does // Generally, poll() is most useful in tests. Often, you may want to verify that a promise does
// not resolve until some specific event occurs. To do so, poll() the promise before the event to // not resolve until some specific event occurs. To do so, poll() the promise before the event to
// verify it isn't resolved, then trigger the event, the poll() again to verify that it resolves. // verify it isn't resolved, then trigger the event, then poll() again to verify that it resolves.
// The first poll() verifies that the promise doesn't resolve early, which would otherwise be // The first poll() verifies that the promise doesn't resolve early, which would otherwise be
// hard to do deterministically. The second poll() allows you to check that the promise has // hard to do deterministically. The second poll() allows you to check that the promise has
// resolved and avoid a wait() that might deadlock in the case that it hasn't. // resolved and avoid a wait() that might deadlock in the case that it hasn't.
......
...@@ -2355,16 +2355,9 @@ KJ_TEST("newHttpService from HttpClient WebSockets disconnect") { ...@@ -2355,16 +2355,9 @@ KJ_TEST("newHttpService from HttpClient WebSockets disconnect") {
// ----------------------------------------------------------------------------- // -----------------------------------------------------------------------------
// TODO(now): Test NetworkAddressHttpClient:
// - Serial requests open only one connection.
// - Parallel requests open multiple connections.
// - Connections time out.
class CountingIoStream final: public kj::AsyncIoStream { class CountingIoStream final: public kj::AsyncIoStream {
// An AsyncIoStream which waits for a promise to resolve then forwards all calls to the promised // An AsyncIoStream wrapper which decrements a counter when destroyed (allowing us to count how
// stream. // many connections are open).
//
// TODO(cleanup): Make this more broadly available.
public: public:
CountingIoStream(kj::Own<kj::AsyncIoStream> inner, uint& count) CountingIoStream(kj::Own<kj::AsyncIoStream> inner, uint& count)
...@@ -2668,7 +2661,7 @@ KJ_TEST("HttpClient multi host") { ...@@ -2668,7 +2661,7 @@ KJ_TEST("HttpClient multi host") {
KJ_EXPECT(addrCount == 2); KJ_EXPECT(addrCount == 2);
KJ_EXPECT(tlsAddrCount == 1); KJ_EXPECT(tlsAddrCount == 1);
// Multipre requests in parallel forces more connections to that host. // Multiple requests in parallel forces more connections to that host.
auto promise1 = doRequest(false, port1); auto promise1 = doRequest(false, port1);
auto promise2 = doRequest(false, port1); auto promise2 = doRequest(false, port1);
promise1.wait(io.waitScope); promise1.wait(io.waitScope);
...@@ -2680,7 +2673,7 @@ KJ_TEST("HttpClient multi host") { ...@@ -2680,7 +2673,7 @@ KJ_TEST("HttpClient multi host") {
// Let everything expire. // Let everything expire.
clientTimer.advanceTo(clientTimer.now() + clientSettings.idleTimout * 2); clientTimer.advanceTo(clientTimer.now() + clientSettings.idleTimout * 2);
kj::Promise<void>(kj::NEVER_DONE).poll(io.waitScope); io.waitScope.poll();
KJ_EXPECT(count == 0); KJ_EXPECT(count == 0);
KJ_EXPECT(tlsCount == 0); KJ_EXPECT(tlsCount == 0);
KJ_EXPECT(addrCount == 0); KJ_EXPECT(addrCount == 0);
......
...@@ -2929,15 +2929,7 @@ private: ...@@ -2929,15 +2929,7 @@ private:
~RefcountedClient() noexcept(false) { ~RefcountedClient() noexcept(false) {
--parent.activeConnectionCount; --parent.activeConnectionCount;
KJ_IF_MAYBE(exception, kj::runCatchingExceptions([&]() { KJ_IF_MAYBE(exception, kj::runCatchingExceptions([&]() {
// Only return the connection to the pool if it is reusable. parent.returnClientToAvailable(kj::mv(client));
if (client->canReuse()) {
parent.availableClients.push_back(AvailableClient {
kj::mv(client), parent.timer.now() + parent.settings.idleTimout
});
}
// Call this either way because it also signals onDrained().
parent.ensureTimetoutsScheduled();
})) { })) {
KJ_LOG(ERROR, *exception); KJ_LOG(ERROR, *exception);
} }
...@@ -2964,7 +2956,15 @@ private: ...@@ -2964,7 +2956,15 @@ private:
} }
} }
void ensureTimetoutsScheduled() { void returnClientToAvailable(kj::Own<HttpClientImpl> client) {
// Only return the connection to the pool if it is reusable.
if (client->canReuse()) {
availableClients.push_back(AvailableClient {
kj::mv(client), timer.now() + settings.idleTimout
});
}
// Call this either way because it also signals onDrained().
if (!timeoutsScheduled) { if (!timeoutsScheduled) {
timeoutsScheduled = true; timeoutsScheduled = true;
timeoutTask = applyTimeouts(); timeoutTask = applyTimeouts();
......
...@@ -572,7 +572,7 @@ struct HttpClientSettings { ...@@ -572,7 +572,7 @@ struct HttpClientSettings {
// omitted. The WebSocket protocol uses random values to avoid triggering flaws (including // omitted. The WebSocket protocol uses random values to avoid triggering flaws (including
// security flaws) in certain HTTP proxy software. Specifically, entropy is used to generate the // security flaws) in certain HTTP proxy software. Specifically, entropy is used to generate the
// `Sec-WebSocket-Key` header and to generate frame masks. If you know that there are no broken // `Sec-WebSocket-Key` header and to generate frame masks. If you know that there are no broken
// or vulnerable proxies between you and the server, you can provide an dummy entropy source that // or vulnerable proxies between you and the server, you can provide a dummy entropy source that
// doesn't generate real entropy (e.g. returning the same value every time). Otherwise, you must // doesn't generate real entropy (e.g. returning the same value every time). Otherwise, you must
// provide a cryptographically-random entropy source. // provide a cryptographically-random entropy source.
}; };
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment