From d3c25b8ea2c5bab449983ae61fbb1aa8c1832301 Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Tue, 8 Feb 2022 15:01:09 +0330 Subject: [PATCH] LibCore: Make BufferedHelper::read() block at most once per read The previous method could block multiple times, leading to a completely stuck/deadlocked read() call, and it could also consume data without telling the user about it, which is Not A Good Thing :tm:. This patch makes it block at most once, and fixes loading HTTP pages with LibHTTP :^) --- Userland/Libraries/LibCore/Stream.h | 38 +++++++++++++++++------------ 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/Userland/Libraries/LibCore/Stream.h b/Userland/Libraries/LibCore/Stream.h index dceb631de8..d8bcb07dd0 100644 --- a/Userland/Libraries/LibCore/Stream.h +++ b/Userland/Libraries/LibCore/Stream.h @@ -521,6 +521,10 @@ public: if (!buffer.size()) return Error::from_errno(ENOBUFS); + // Fill the internal buffer if it has run dry. + if (m_buffered_size == 0) + TRY(populate_read_buffer()); + // Let's try to take all we can from the buffer first. size_t buffer_nread = 0; if (m_buffered_size > 0) { @@ -539,20 +543,7 @@ public: m_buffered_size -= amount_to_take; } - // If the buffer satisfied the request, then we need not continue. - if (buffer_nread == buffer.size()) { - return buffer_nread; - } - - // Otherwise, let's try an extra read just in case there's something - // in our receive buffer. - auto stream_nread = TRY(stream().read(buffer.slice(buffer_nread))); - - // Fill the internal buffer if it has run dry. - if (m_buffered_size == 0) - TRY(populate_read_buffer()); - - return buffer_nread + stream_nread; + return buffer_nread; } // Reads into the buffer until \n is encountered. @@ -703,8 +694,23 @@ private: return ReadonlyBytes {}; auto fillable_slice = m_buffer.span().slice(m_buffered_size); - auto nread = TRY(stream().read(fillable_slice)); - m_buffered_size += nread; + size_t nread = 0; + do { + auto result = stream().read(fillable_slice); + if (result.is_error()) { + if (!result.error().is_errno()) + return result.error(); + if (result.error().code() == EINTR) + continue; + if (result.error().code() == EAGAIN) + break; + return result.error(); + } + auto read_size = result.value(); + m_buffered_size += read_size; + nread += read_size; + break; + } while (true); return fillable_slice.slice(0, nread); }