Skip to content

Commit

Permalink
Fix some timeout edge cases
Browse files Browse the repository at this point in the history
- Explicitely use the frontend timeout on keep-alive (may introduce a
  specific one later)
- Put timeout responsibility on the backend whenever we read the first
  bytes
- Mark as terminated GET requests with no length (we assume no body)
- Properly reset the HttpContext of the session

Signed-off-by: Eloi DEMOLIS <eloi.demolis@clever-cloud.com>
  • Loading branch information
Wonshtrum committed Mar 18, 2024
1 parent 123cb1e commit d20e759
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 8 deletions.
1 change: 0 additions & 1 deletion bin/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ pub unsafe fn get_executable_path() -> Result<String, UtilError> {
Ok(path_str)
}


#[cfg(target_os = "macos")]
extern "C" {
pub fn _NSGetExecutablePath(buf: *mut libc::c_char, size: *mut u32) -> i32;
Expand Down
16 changes: 16 additions & 0 deletions lib/src/protocol/kawa_h1/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ impl HttpContext {
.map(ToOwned::to_owned);
}

if self.method == Some(Method::Get) && request.body_size == kawa::BodySize::Empty {
request.parsing_phase = kawa::ParsingPhase::Terminated;
}

let public_ip = self.public_address.ip();
let public_port = self.public_address.port();
let proto = match self.protocol {
Expand Down Expand Up @@ -337,4 +341,16 @@ impl HttpContext {
val: kawa::Store::from_string(self.id.to_string()),
}));
}

pub fn reset(&mut self) {
self.keep_alive_backend = true;
self.keep_alive_frontend = true;
self.sticky_session_found = None;
self.method = None;
self.authority = None;
self.path = None;
self.status = None;
self.reason = None;
self.user_agent = None;
}
}
25 changes: 18 additions & 7 deletions lib/src/protocol/kawa_h1/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,8 @@ impl<Front: SocketHandler, L: ListenerHandler + L7ListenerHandler> Http<Front, L
/// Reset the connection in case of keep-alive to be ready for the next request
pub fn reset(&mut self) {
trace!("==============reset");
self.context.keep_alive_frontend = true;
self.context.keep_alive_backend = true;
self.context.sticky_session_found = None;
self.context.id = Ulid::generate();
self.context.reset();

self.request_stream.clear();
self.response_stream.clear();
Expand All @@ -239,8 +237,9 @@ impl<Front: SocketHandler, L: ListenerHandler + L7ListenerHandler> Http<Front, L

// reset the front timeout and cancel the back timeout while we are
// waiting for a new request
self.container_frontend_timeout.reset();
self.container_backend_timeout.cancel();
self.container_frontend_timeout
.set_duration(self.configured_frontend_timeout);
self.frontend_readiness.interest = Ready::READABLE | Ready::HUP | Ready::ERROR;
self.backend_readiness.interest = Ready::HUP | Ready::ERROR;

Expand Down Expand Up @@ -528,7 +527,7 @@ impl<Front: SocketHandler, L: ListenerHandler + L7ListenerHandler> Http<Front, L
return match (
self.context.keep_alive_frontend,
self.context.keep_alive_backend,
response_length_known
response_length_known,
) {
(true, true, true) => {
debug!("{} keep alive front/back", self.log_context());
Expand Down Expand Up @@ -636,6 +635,12 @@ impl<Front: SocketHandler, L: ListenerHandler + L7ListenerHandler> Http<Front, L
);
self.print_state(self.protocol_string());
}
// In case both the response happens while the request is not tagged as "terminated",
// we place the timeout responsibility on the backend.
// This can happen when:
// - kawa fails to detect a properly terminated request (e.g. a GET request with no body and no length)
// - the response can start before the end of the request (e.g. stream processing like compression)
self.container_frontend_timeout.cancel();

if let SessionStatus::DefaultAnswer(_, _, _) = self.status {
error!(
Expand Down Expand Up @@ -1761,15 +1766,20 @@ impl<Front: SocketHandler, L: ListenerHandler + L7ListenerHandler> SessionState
if self.frontend_token == token {
self.container_frontend_timeout.triggered();
return match self.timeout_status() {
// we do not have a complete answer
TimeoutStatus::Request => {
self.set_answer(DefaultAnswerStatus::Answer408, None);
self.writable(metrics)
}
// we have a complete answer but the response did not start
TimeoutStatus::WaitingForResponse => {
self.set_answer(DefaultAnswerStatus::Answer504, None);
self.set_answer(DefaultAnswerStatus::Answer408, None);
self.writable(metrics)
}
TimeoutStatus::Response => StateResult::CloseSession,
// we have a complete answer and the start of a response, but the request was not tagged as terminated
// for now we place responsibility of timeout on the backend in those cases, so we ignore this
TimeoutStatus::Response => StateResult::Continue,
// timeout in keep-alive, simply close the connection
TimeoutStatus::WaitingForNewRequest => StateResult::CloseSession,
};
}
Expand All @@ -1796,6 +1806,7 @@ impl<Front: SocketHandler, L: ListenerHandler + L7ListenerHandler> SessionState
);
StateResult::CloseSession
}
// in keep-alive, we place responsibility of timeout on the frontend, so we ignore this
TimeoutStatus::WaitingForNewRequest => StateResult::Continue,
};
}
Expand Down

0 comments on commit d20e759

Please sign in to comment.