ProtocolServer: Avoid blocking all downloads when client stops reading

Fixes SerenityOS#4668.

Libraries/LibGemini/Job.cpp

@@ -165,10 +165,12 @@ void Job::finish_up()
  m_state = State::Finished;
  flush_received_buffers();
  if (m_received_size != 0) {
- // FIXME: What do we do? ignore it?
- // "Transmission failed" is not strictly correct, but let's roll with it for now.
+ // We have to wait for the client to consume all the downloaded data
+ // before we can actually call `did_finish`. in a normal flow, this should
+ // never be hit since the client is reading as we are writing, unless there
+ // are too many concurrent downloads going on.
  deferred_invoke([this](auto&) {
- did_fail(Error::TransmissionFailed);
+ finish_up();
  });
  return;
  }

Libraries/LibHTTP/Job.cpp

@@ -376,10 +376,12 @@ void Job::finish_up()
 
  flush_received_buffers();
  if (m_buffered_size != 0) {
- // FIXME: What do we do? ignore it?
- // "Transmission failed" is not strictly correct, but let's roll with it for now.
+ // We have to wait for the client to consume all the downloaded data
+ // before we can actually call `did_finish`. in a normal flow, this should
+ // never be hit since the client is reading as we are writing, unless there
+ // are too many concurrent downloads going on.
  deferred_invoke([this](auto&) {
- did_fail(Error::TransmissionFailed);
+ finish_up();
  });
  return;
  }

Services/ProtocolServer/GeminiProtocol.cpp

@@ -28,6 +28,7 @@
 #include <LibGemini/GeminiRequest.h>
 #include <ProtocolServer/GeminiDownload.h>
 #include <ProtocolServer/GeminiProtocol.h>
+#include <fcntl.h>
 
 namespace ProtocolServer {
 
@@ -45,17 +46,15 @@ OwnPtr<Download> GeminiProtocol::start_download(ClientConnection& client, const
  Gemini::GeminiRequest request;
  request.set_url(url);
 
- int fd_pair[2] { 0 };
- if (pipe(fd_pair) != 0) {
- auto saved_errno = errno;
- dbgln("Protocol: pipe() failed: {}", strerror(saved_errno));
+ auto pipe_result = get_pipe_for_download();
+ if (pipe_result.is_error())
  return nullptr;
- }
- auto output_stream = make<OutputFileStream>(fd_pair[1]);
+
+ auto output_stream = make<OutputFileStream>(pipe_result.value().write_fd);
  output_stream->make_unbuffered();
  auto job = Gemini::GeminiJob::construct(request, *output_stream);
  auto download = GeminiDownload::create_with_job({}, client, (Gemini::GeminiJob&)*job, move(output_stream));
- download->set_download_fd(fd_pair[0]);
+ download->set_download_fd(pipe_result.value().read_fd);
  job->start();
  return download;
 }

Services/ProtocolServer/HttpProtocol.cpp

@@ -28,7 +28,6 @@
 #include <LibHTTP/HttpRequest.h>
 #include <ProtocolServer/HttpDownload.h>
 #include <ProtocolServer/HttpProtocol.h>
-#include <fcntl.h>
 
 namespace ProtocolServer {
 
@@ -52,18 +51,15 @@ OwnPtr<Download> HttpProtocol::start_download(ClientConnection& client, const St
  request.set_headers(headers);
  request.set_body(body);
 
- int fd_pair[2] { 0 };
- if (pipe(fd_pair) != 0) {
- auto saved_errno = errno;
- dbgln("Protocol: pipe() failed: {}", strerror(saved_errno));
+ auto pipe_result = get_pipe_for_download();
+ if (pipe_result.is_error())
  return nullptr;
- }
 
- auto output_stream = make<OutputFileStream>(fd_pair[1]);
+ auto output_stream = make<OutputFileStream>(pipe_result.value().write_fd);
  output_stream->make_unbuffered();
  auto job = HTTP::HttpJob::construct(request, *output_stream);
  auto download = HttpDownload::create_with_job({}, client, (HTTP::HttpJob&)*job, move(output_stream));
- download->set_download_fd(fd_pair[0]);
+ download->set_download_fd(pipe_result.value().read_fd);
  job->start();
  return download;
 }

Services/ProtocolServer/HttpsProtocol.cpp

@@ -51,17 +51,15 @@ OwnPtr<Download> HttpsProtocol::start_download(ClientConnection& client, const S
  request.set_headers(headers);
  request.set_body(body);
 
- int fd_pair[2] { 0 };
- if (pipe(fd_pair) != 0) {
- auto saved_errno = errno;
- dbgln("Protocol: pipe() failed: {}", strerror(saved_errno));
+ auto pipe_result = get_pipe_for_download();
+ if (pipe_result.is_error())
  return nullptr;
- }
- auto output_stream = make<OutputFileStream>(fd_pair[1]);
+
+ auto output_stream = make<OutputFileStream>(pipe_result.value().write_fd);
  output_stream->make_unbuffered();
  auto job = HTTP::HttpsJob::construct(request, *output_stream);
  auto download = HttpsDownload::create_with_job({}, client, (HTTP::HttpsJob&)*job, move(output_stream));
- download->set_download_fd(fd_pair[0]);
+ download->set_download_fd(pipe_result.value().read_fd);
  job->start();
  return download;
 }

Services/ProtocolServer/Protocol.cpp

@@ -26,6 +26,8 @@
 
 #include <AK/HashMap.h>
 #include <ProtocolServer/Protocol.h>
+#include <fcntl.h>
+#include <string.h>
 
 namespace ProtocolServer {
 
@@ -50,4 +52,16 @@ Protocol::~Protocol()
  ASSERT_NOT_REACHED();
 }
 
+Result<Protocol::Pipe, String> Protocol::get_pipe_for_download()
+{
+ int fd_pair[2] { 0 };
+ if (pipe(fd_pair) != 0) {
+ auto saved_errno = errno;
+ dbgln("Protocol: pipe() failed: {}", strerror(saved_errno));
+ return String { strerror(saved_errno) };
+ }
+ fcntl(fd_pair[1], F_SETFL, fcntl(fd_pair[1], F_GETFL) | O_NONBLOCK);
+ return Pipe { fd_pair[0], fd_pair[1] };
+}
+
 }

Services/ProtocolServer/Protocol.h

@@ -27,6 +27,7 @@
 #pragma once
 
 #include <AK/RefPtr.h>
+#include <AK/Result.h>
 #include <AK/URL.h>
 #include <ProtocolServer/Forward.h>
 
@@ -43,6 +44,11 @@ class Protocol {
 
 protected:
  explicit Protocol(const String& name);
+ struct Pipe {
+ int read_fd { -1 };
+ int write_fd { -1 };
+ };
+ static Result<Pipe, String> get_pipe_for_download();
 
 private:
  String m_name;