RequestServer+LibHTTP: Cancel requests on client exit

That usually happens in "exceptional" states when the client exits
unexpectedly (crash, force quit mid-load, etc), leading to the job flush
timer firing and attempting to write to a nonexistent object (the
client).

This commit makes RS simply cancel such jobs; cancelled jobs in this
state simply go away instead of sending notifications around.
This commit is contained in:
Ali Mohammad Pur 2024-07-28 00:22:58 +02:00 committed by Ali Mohammad Pur
commit 4211639e45
Notes: github-actions[bot] 2024-07-28 11:46:35 +00:00
8 changed files with 25 additions and 7 deletions

View file

@ -576,6 +576,13 @@ void Job::finish_up()
{
VERIFY(!m_has_scheduled_finish);
m_state = State::Finished;
if (is_cancelled()) {
stop_timer();
m_has_scheduled_finish = true;
return;
}
if (!m_can_stream_response) {
auto maybe_flattened_buffer = ByteBuffer::create_uninitialized(m_buffered_size);
if (maybe_flattened_buffer.is_error())

View file

@ -101,7 +101,7 @@ struct JobData {
#endif
template<typename T>
static JobData create(NonnullRefPtr<T> job, [[maybe_unused]] URL::URL url)
static JobData create(WeakPtr<T> job, [[maybe_unused]] URL::URL url)
{
return JobData {
[job](auto& socket) { job->start(socket); },

View file

@ -46,6 +46,14 @@ ConnectionFromClient::ConnectionFromClient(NonnullOwnPtr<Core::LocalSocket> sock
s_connections.set(client_id(), *this);
}
ConnectionFromClient::~ConnectionFromClient()
{
m_requests.with_locked([](HashMap<i32, OwnPtr<Request>>& map) {
for (auto& entry : map)
entry.value->cancel();
});
}
class Job : public RefCounted<Job>
, public Weakable<Job> {
public:
@ -152,10 +160,9 @@ void ConnectionFromClient::worker_do_work(Work work)
return;
}
auto job = Job::ensure(url);
dbgln("EnsureConnection: Pre-connect to {}", url);
auto do_preconnect = [&](auto& cache) {
ConnectionCache::ensure_connection(cache, url, job);
auto do_preconnect = [=, job = Job::ensure(url)](auto& cache) {
ConnectionCache::ensure_connection(cache, url, job->make_weak_ptr<Job>());
};
if (url.scheme() == "http"sv)

View file

@ -23,7 +23,7 @@ class ConnectionFromClient final
C_OBJECT(ConnectionFromClient);
public:
~ConnectionFromClient() override = default;
~ConnectionFromClient() override;
virtual void die() override;

View file

@ -99,10 +99,10 @@ OwnPtr<Request> start_request(TBadgedProtocol&& protocol, i32 request_id, Connec
auto output_stream = MUST(Core::File::adopt_fd(pipe_result.value().write_fd, Core::File::OpenMode::Write));
auto job = TJob::construct(move(request), *output_stream);
auto protocol_request = TRequest::create_with_job(forward<TBadgedProtocol>(protocol), client, (TJob&)*job, move(output_stream), request_id);
auto protocol_request = TRequest::create_with_job(forward<TBadgedProtocol>(protocol), client, static_cast<TJob&>(*job), move(output_stream), request_id);
protocol_request->set_request_fd(pipe_result.value().read_fd);
Core::deferred_invoke([=] {
Core::deferred_invoke([=, job = job->template make_weak_ptr<TJob>()] {
if constexpr (IsSame<typename TBadgedProtocol::Type, HttpsProtocol>)
ConnectionCache::ensure_connection(ConnectionCache::g_tls_connection_cache, url, job, proxy_data);
else

View file

@ -23,6 +23,7 @@ public:
HTTP::Job const& job() const { return m_job; }
virtual URL::URL url() const override { return m_job->url(); }
virtual void cancel() override { m_job->cancel(); }
private:
explicit HttpRequest(ConnectionFromClient&, NonnullRefPtr<HTTP::Job>, NonnullOwnPtr<Core::File>&&, i32);

View file

@ -22,6 +22,7 @@ public:
HTTP::HttpsJob const& job() const { return m_job; }
virtual URL::URL url() const override { return m_job->url(); }
virtual void cancel() override { m_job->cancel(); }
private:
explicit HttpsRequest(ConnectionFromClient&, NonnullRefPtr<HTTP::HttpsJob>, NonnullOwnPtr<Core::File>&&, i32);

View file

@ -34,6 +34,8 @@ public:
void set_request_fd(int fd) { m_request_fd = fd; }
int request_fd() const { return m_request_fd; }
virtual void cancel() = 0;
void did_finish(bool success);
void did_progress(Optional<u64> total_size, u64 downloaded_size);
void set_status_code(u32 status_code) { m_status_code = status_code; }