LibCore+LibTLS: Don't keep a "ready to write" notifier on all Sockets

The "ready to write" notifier we set up in generic socket connection is
really only meant to detect a successful connection. Once we have a TCP
connection, for example, it will fire on every event loop iteration.

This was causing IRC Client to max out the CPU by getting this no-op
notifier callback over and over.

Since this was only used by TLSv12, I changed that code to create its
own notifier instead. It might be possible to improve TLS performance
by only processing writes when actually needed, but I didn't look very
closely at that for this patch. :^)

5 files changed, 27 insertions, 20 deletions

diff --git a/Libraries/LibCore/Socket.cpp b/Libraries/LibCore/Socket.cpp
index f1b261f852..5ce1e42ae8 100644
--- a/Libraries/LibCore/Socket.cpp
+++ b/Libraries/LibCore/Socket.cpp
@@ -127,11 +127,13 @@ bool Socket::common_connect(const struct sockaddr* addr, socklen_t addrlen)
         if (!m_connected) {
             m_connected = true;
             ensure_read_notifier();
+            if (m_notifier) {
+                m_notifier->remove_from_parent();
+                m_notifier = nullptr;
+            }
             if (on_connected)
                 on_connected();
         }
-        if (on_ready_to_write)
-            on_ready_to_write();
     };
     int rc = ::connect(fd(), addr, addrlen);
     if (rc < 0) {
diff --git a/Libraries/LibCore/Socket.h b/Libraries/LibCore/Socket.h
index 3985dc8bde..e6803fbb40 100644
--- a/Libraries/LibCore/Socket.h
+++ b/Libraries/LibCore/Socket.h
@@ -63,7 +63,6 @@ public:
 
     Function<void()> on_connected;
     Function<void()> on_ready_to_read;
-    Function<void()> on_ready_to_write;
 
 protected:
     Socket(Type, Object* parent);
diff --git a/Libraries/LibTLS/Socket.cpp b/Libraries/LibTLS/Socket.cpp
index 6b0488104d..1662d75abf 100644
--- a/Libraries/LibTLS/Socket.cpp
+++ b/Libraries/LibTLS/Socket.cpp
@@ -142,7 +142,8 @@ bool TLSv12::common_connect(const struct sockaddr* saddr, socklen_t length)
                 if (on_tls_ready_to_read)
                     on_tls_ready_to_read(*this);
         };
-        Core::Socket::on_ready_to_write = [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";
diff --git a/Libraries/LibTLS/TLSv12.cpp b/Libraries/LibTLS/TLSv12.cpp
index 72a6186e93..23c5ab4e76 100644
--- a/Libraries/LibTLS/TLSv12.cpp
+++ b/Libraries/LibTLS/TLSv12.cpp
@@ -641,4 +641,21 @@ void TLSv12::try_disambiguate_error() const
         break;
     }
 }
+
+TLSv12::TLSv12(Core::Object* parent, Version version)
+    : Core::Socket(Core::Socket::Type::TCP, parent)
+{
+    m_context.version = version;
+    m_context.is_server = false;
+    m_context.tls_buffer = ByteBuffer::create_uninitialized(0);
+    int fd = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);
+    if (fd < 0) {
+        set_error(errno);
+    } else {
+        set_fd(fd);
+        set_mode(IODevice::ReadWrite);
+        set_error(0);
+    }
+}
+
 }
diff --git a/Libraries/LibTLS/TLSv12.h b/Libraries/LibTLS/TLSv12.h
index ff4d844190..9c7425d607 100644
--- a/Libraries/LibTLS/TLSv12.h
+++ b/Libraries/LibTLS/TLSv12.h
@@ -298,22 +298,6 @@ struct Context {
 class TLSv12 : public Core::Socket {
     C_OBJECT(TLSv12)
 public:
-    explicit TLSv12(Core::Object* parent, Version version = Version::V12)
-        : Core::Socket(Core::Socket::Type::TCP, parent)
-    {
-        m_context.version = version;
-        m_context.is_server = false;
-        m_context.tls_buffer = ByteBuffer::create_uninitialized(0);
-        int fd = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);
-        if (fd < 0) {
-            set_error(errno);
-        } else {
-            set_fd(fd);
-            set_mode(IODevice::ReadWrite);
-            set_error(0);
-        }
-    }
-
     ByteBuffer& write_buffer() { return m_context.tls_buffer; }
     bool is_established() const { return m_context.connection_status == ConnectionStatus::Established; }
     virtual bool connect(const String&, int) override;
@@ -364,6 +348,8 @@ public:
     Function<void()> on_tls_finished;
 
 private:
+    explicit TLSv12(Core::Object* parent, Version version = Version::V12);
+
     virtual bool common_connect(const struct sockaddr*, socklen_t) override;
 
     void consume(const ByteBuffer& record);
@@ -476,6 +462,8 @@ private:
 
     OwnPtr<Crypto::Cipher::AESCipher::CBCMode> m_aes_local;
     OwnPtr<Crypto::Cipher::AESCipher::CBCMode> m_aes_remote;
+
+    RefPtr<Core::Notifier> m_write_notifier;
 };
 
 namespace Constants {