From 379cb061d73ad92d978f05e518b28520ddd5231c Mon Sep 17 00:00:00 2001 From: AnotherTest Date: Tue, 19 May 2020 19:03:57 +0430 Subject: [PATCH] LibTLS: Only try to flush data when needed This patchset drops the write notifier, and schedules writes only when necessary. As a result, the CPU utilisation no longer spikes to the skies :^) --- Libraries/LibTLS/Record.cpp | 6 ++ Libraries/LibTLS/Socket.cpp | 109 ++++++++++++++++++------------------ Libraries/LibTLS/TLSv12.h | 5 +- 3 files changed, 64 insertions(+), 56 deletions(-) diff --git a/Libraries/LibTLS/Record.cpp b/Libraries/LibTLS/Record.cpp index 448e6cab38f..423411cb735 100644 --- a/Libraries/LibTLS/Record.cpp +++ b/Libraries/LibTLS/Record.cpp @@ -34,6 +34,12 @@ namespace TLS { void TLSv12::write_packet(ByteBuffer& packet) { m_context.tls_buffer.append(packet.data(), packet.size()); + if (!m_has_scheduled_write_flush && m_context.connection_status > ConnectionStatus::Disconnected) { +#ifdef TLS_DEBUG + dbg() << "Scheduling write of " << m_context.tls_buffer.size(); +#endif + deferred_invoke([this](auto&) { write_into_socket(); }); + } } void TLSv12::update_packet(ByteBuffer& packet) diff --git a/Libraries/LibTLS/Socket.cpp b/Libraries/LibTLS/Socket.cpp index 1662d75abf5..95790fa412c 100644 --- a/Libraries/LibTLS/Socket.cpp +++ b/Libraries/LibTLS/Socket.cpp @@ -113,67 +113,15 @@ bool TLSv12::common_connect(const struct sockaddr* saddr, socklen_t length) Core::Socket::on_connected = [this] { Core::Socket::on_ready_to_read = [this] { - if (!Core::Socket::is_open() || !Core::Socket::is_connected() || Core::Socket::eof()) { - // an abrupt closure (the server is a jerk) - dbg() << "Socket not open, assuming abrupt closure"; - m_context.connection_finished = true; - } - if (m_context.critical_error) { - dbg() << "READ CRITICAL ERROR " << m_context.critical_error << " :("; - if (on_tls_error) - on_tls_error((AlertDescription)m_context.critical_error); + if (!check_connection_state(true)) return; - } - if (m_context.application_buffer.size() == 0 && m_context.connection_finished) { - if (on_tls_finished) - on_tls_finished(); - if (m_context.tls_buffer.size()) { - dbg() << "connection closed without finishing data transfer, " << m_context.tls_buffer.size() << " bytes still in buffer"; - } else { - m_context.connection_finished = false; - } - if (!m_context.application_buffer.size()) - m_context.connection_status = ConnectionStatus::Disconnected; - return; - } flush(); - consume(Core::Socket::read(4096)); // FIXME: how much is proper? + consume(Core::Socket::read(4096)); if (is_established() && m_context.application_buffer.size()) if (on_tls_ready_to_read) on_tls_ready_to_read(*this); }; - m_write_notifier = Core::Notifier::construct(fd(), Core::Notifier::Event::Write); - m_write_notifier->on_ready_to_write = [this] { - if (!Core::Socket::is_open() || !Core::Socket::is_connected() || Core::Socket::eof()) { - // an abrupt closure (the server is a jerk) - dbg() << "Socket not open, assuming abrupt closure"; - m_context.connection_finished = true; - } - if (m_context.critical_error) { - dbg() << "WRITE CRITICAL ERROR " << m_context.critical_error << " :("; - if (on_tls_error) - on_tls_error((AlertDescription)m_context.critical_error); - return; - } - if (m_context.connection_finished) { - if (on_tls_finished) - on_tls_finished(); - if (m_context.tls_buffer.size()) { - dbg() << "connection closed without finishing data transfer, " << m_context.tls_buffer.size() << " bytes still in buffer"; - } else { - m_context.connection_finished = false; - dbg() << "FINISHED"; - } - if (!m_context.application_buffer.size()) { - m_context.connection_status = ConnectionStatus::Disconnected; - return; - } - } - flush(); - if (is_established() && !m_context.application_buffer.size()) // hey client, you still have stuff to read... - if (on_tls_ready_to_write) - on_tls_ready_to_write(*this); - }; + write_into_socket(); if (on_tls_connected) on_tls_connected(); }; @@ -184,6 +132,57 @@ bool TLSv12::common_connect(const struct sockaddr* saddr, socklen_t length) return true; } +void TLSv12::write_into_socket() +{ +#ifdef TLS_DEBUG + dbg() << "Flushing cached records: " << m_context.tls_buffer.size() << " established? " << is_established(); +#endif + m_has_scheduled_write_flush = false; + if (!check_connection_state(false)) + return; + flush(); + + if (!is_established()) { + deferred_invoke([this](auto&) { write_into_socket(); }); + m_has_scheduled_write_flush = true; + return; + } + + if (is_established() && !m_context.application_buffer.size()) // hey client, you still have stuff to read... + if (on_tls_ready_to_write) + on_tls_ready_to_write(*this); +} + +bool TLSv12::check_connection_state(bool read) +{ + if (!Core::Socket::is_open() || !Core::Socket::is_connected() || Core::Socket::eof()) { + // an abrupt closure (the server is a jerk) + dbg() << "Socket not open, assuming abrupt closure"; + m_context.connection_finished = true; + } + if (m_context.critical_error) { + dbg() << "WRITE CRITICAL ERROR " << m_context.critical_error << " :("; + if (on_tls_error) + on_tls_error((AlertDescription)m_context.critical_error); + return false; + } + if (((read && m_context.application_buffer.size() == 0) || !read) && m_context.connection_finished) { + if (on_tls_finished) + on_tls_finished(); + if (m_context.tls_buffer.size()) { + dbg() << "connection closed without finishing data transfer, " << m_context.tls_buffer.size() << " bytes still in buffer"; + } else { + m_context.connection_finished = false; + dbg() << "FINISHED"; + } + if (!m_context.application_buffer.size()) { + m_context.connection_status = ConnectionStatus::Disconnected; + return false; + } + } + return true; +} + bool TLSv12::flush() { auto out_buffer = write_buffer().data(); diff --git a/Libraries/LibTLS/TLSv12.h b/Libraries/LibTLS/TLSv12.h index 9c7425d607b..efd13943899 100644 --- a/Libraries/LibTLS/TLSv12.h +++ b/Libraries/LibTLS/TLSv12.h @@ -375,6 +375,9 @@ private: void build_random(PacketBuilder&); bool flush(); + void write_into_socket(); + + bool check_connection_state(bool read); ssize_t handle_hello(const ByteBuffer& buffer, WritePacketStage&); ssize_t handle_finished(const ByteBuffer& buffer, WritePacketStage&); @@ -463,7 +466,7 @@ private: OwnPtr m_aes_local; OwnPtr m_aes_remote; - RefPtr m_write_notifier; + bool m_has_scheduled_write_flush = false; }; namespace Constants {