mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-07-29 04:09:13 +00:00
RequestServer: Handle client disappearance more gracefully
Without these fixes, RequestServer was likely to crash if the client crashed (e.g. WebContent). This was because there was no error handling for when writing to the client failed. This is particularly an issue because RequestServer has shared instances, so it would then crash every other client of RequestServer. Then, because another RequestServer instance is not currently spun up, it becomes impossible to start any clients that need a RequestServer instance. Recreating a RequestServer should also be handled, but that's not in the scope of this change. We can tell curl that we failed to write data to the client and that the request should be aborted by returning `CURL_WRITEFUNC_ERROR` from the write callback. It is also possible for requests to be destroyed with buffered data, which is normal to happen if the client disappears (i.e. ConnectionFromClient is destroyed) or the request is cancelled by the client. We log a warning in case this is not expected, to assist with debugging related issues.
This commit is contained in:
parent
01ede6cc58
commit
2dead9231d
Notes:
github-actions[bot]
2025-06-13 15:05:07 +00:00
Author: https://github.com/Lubrsi
Commit: 2dead9231d
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/5076
Reviewed-by: https://github.com/gmta ✅
1 changed files with 22 additions and 7 deletions
|
@ -128,7 +128,9 @@ struct ConnectionFromClient::ActiveRequest : public Weakable<ActiveRequest> {
|
|||
{
|
||||
write_notifier->set_enabled(false);
|
||||
write_notifier->on_activation = [this] {
|
||||
write_queued_bytes_without_blocking();
|
||||
if (auto maybe_error = write_queued_bytes_without_blocking(); maybe_error.is_error()) {
|
||||
dbgln("Warning: Failed to write buffered request data (it's likely the client disappeared): {}", maybe_error.error());
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
|
@ -142,7 +144,7 @@ struct ConnectionFromClient::ActiveRequest : public Weakable<ActiveRequest> {
|
|||
});
|
||||
}
|
||||
|
||||
void write_queued_bytes_without_blocking()
|
||||
ErrorOr<void> write_queued_bytes_without_blocking()
|
||||
{
|
||||
Vector<u8> bytes_to_send;
|
||||
bytes_to_send.resize(send_buffer.used_buffer_size());
|
||||
|
@ -150,16 +152,18 @@ struct ConnectionFromClient::ActiveRequest : public Weakable<ActiveRequest> {
|
|||
auto result = Core::System::write(this->writer_fd, bytes_to_send);
|
||||
if (result.is_error()) {
|
||||
if (result.error().code() != EAGAIN) {
|
||||
VERIFY_NOT_REACHED();
|
||||
return result.release_error();
|
||||
}
|
||||
write_notifier->set_enabled(true);
|
||||
return;
|
||||
return {};
|
||||
}
|
||||
|
||||
MUST(send_buffer.discard(result.value()));
|
||||
write_notifier->set_enabled(!send_buffer.is_eof());
|
||||
if (send_buffer.is_eof() && done_fetching)
|
||||
schedule_self_destruction();
|
||||
|
||||
return {};
|
||||
}
|
||||
|
||||
void notify_about_fetching_completion()
|
||||
|
@ -171,7 +175,9 @@ struct ConnectionFromClient::ActiveRequest : public Weakable<ActiveRequest> {
|
|||
|
||||
~ActiveRequest()
|
||||
{
|
||||
VERIFY(send_buffer.is_eof());
|
||||
if (!send_buffer.is_eof()) {
|
||||
dbgln("Warning: Request destroyed with buffered data (it's likely that the client disappeared or the request was cancelled)");
|
||||
}
|
||||
|
||||
if (writer_fd > 0)
|
||||
MUST(Core::System::close(writer_fd));
|
||||
|
@ -235,8 +241,17 @@ size_t ConnectionFromClient::on_data_received(void* buffer, size_t size, size_t
|
|||
|
||||
size_t total_size = size * nmemb;
|
||||
ReadonlyBytes bytes { static_cast<u8 const*>(buffer), total_size };
|
||||
MUST(request->send_buffer.write_some(bytes));
|
||||
request->write_queued_bytes_without_blocking();
|
||||
|
||||
auto maybe_write_error = [&] -> ErrorOr<void> {
|
||||
TRY(request->send_buffer.write_some(bytes));
|
||||
return request->write_queued_bytes_without_blocking();
|
||||
}();
|
||||
|
||||
if (maybe_write_error.is_error()) {
|
||||
dbgln("ConnectionFromClient::on_data_received: Aborting request because error occurred whilst writing data to the client: {}", maybe_write_error.error());
|
||||
return CURL_WRITEFUNC_ERROR;
|
||||
}
|
||||
|
||||
request->downloaded_so_far += total_size;
|
||||
return total_size;
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue