mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 09:02:43 +00:00 
			
		
		
		
	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. :^)
This commit is contained in:
		
							parent
							
								
									9eaf22090f
								
							
						
					
					
						commit
						4b202a3c79
					
				
					 5 changed files with 27 additions and 20 deletions
				
			
		|  | @ -127,11 +127,13 @@ bool Socket::common_connect(const struct sockaddr* addr, socklen_t addrlen) | ||||||
|         if (!m_connected) { |         if (!m_connected) { | ||||||
|             m_connected = true; |             m_connected = true; | ||||||
|             ensure_read_notifier(); |             ensure_read_notifier(); | ||||||
|  |             if (m_notifier) { | ||||||
|  |                 m_notifier->remove_from_parent(); | ||||||
|  |                 m_notifier = nullptr; | ||||||
|  |             } | ||||||
|             if (on_connected) |             if (on_connected) | ||||||
|                 on_connected(); |                 on_connected(); | ||||||
|         } |         } | ||||||
|         if (on_ready_to_write) |  | ||||||
|             on_ready_to_write(); |  | ||||||
|     }; |     }; | ||||||
|     int rc = ::connect(fd(), addr, addrlen); |     int rc = ::connect(fd(), addr, addrlen); | ||||||
|     if (rc < 0) { |     if (rc < 0) { | ||||||
|  |  | ||||||
|  | @ -63,7 +63,6 @@ public: | ||||||
| 
 | 
 | ||||||
|     Function<void()> on_connected; |     Function<void()> on_connected; | ||||||
|     Function<void()> on_ready_to_read; |     Function<void()> on_ready_to_read; | ||||||
|     Function<void()> on_ready_to_write; |  | ||||||
| 
 | 
 | ||||||
| protected: | protected: | ||||||
|     Socket(Type, Object* parent); |     Socket(Type, Object* parent); | ||||||
|  |  | ||||||
|  | @ -142,7 +142,8 @@ bool TLSv12::common_connect(const struct sockaddr* saddr, socklen_t length) | ||||||
|                 if (on_tls_ready_to_read) |                 if (on_tls_ready_to_read) | ||||||
|                     on_tls_ready_to_read(*this); |                     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()) { |             if (!Core::Socket::is_open() || !Core::Socket::is_connected() || Core::Socket::eof()) { | ||||||
|                 // an abrupt closure (the server is a jerk)
 |                 // an abrupt closure (the server is a jerk)
 | ||||||
|                 dbg() << "Socket not open, assuming abrupt closure"; |                 dbg() << "Socket not open, assuming abrupt closure"; | ||||||
|  |  | ||||||
|  | @ -641,4 +641,21 @@ void TLSv12::try_disambiguate_error() const | ||||||
|         break; |         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); | ||||||
|  |     } | ||||||
|  | } | ||||||
|  | 
 | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -298,22 +298,6 @@ struct Context { | ||||||
| class TLSv12 : public Core::Socket { | class TLSv12 : public Core::Socket { | ||||||
|     C_OBJECT(TLSv12) |     C_OBJECT(TLSv12) | ||||||
| public: | 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; } |     ByteBuffer& write_buffer() { return m_context.tls_buffer; } | ||||||
|     bool is_established() const { return m_context.connection_status == ConnectionStatus::Established; } |     bool is_established() const { return m_context.connection_status == ConnectionStatus::Established; } | ||||||
|     virtual bool connect(const String&, int) override; |     virtual bool connect(const String&, int) override; | ||||||
|  | @ -364,6 +348,8 @@ public: | ||||||
|     Function<void()> on_tls_finished; |     Function<void()> on_tls_finished; | ||||||
| 
 | 
 | ||||||
| private: | private: | ||||||
|  |     explicit TLSv12(Core::Object* parent, Version version = Version::V12); | ||||||
|  | 
 | ||||||
|     virtual bool common_connect(const struct sockaddr*, socklen_t) override; |     virtual bool common_connect(const struct sockaddr*, socklen_t) override; | ||||||
| 
 | 
 | ||||||
|     void consume(const ByteBuffer& record); |     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_local; | ||||||
|     OwnPtr<Crypto::Cipher::AESCipher::CBCMode> m_aes_remote; |     OwnPtr<Crypto::Cipher::AESCipher::CBCMode> m_aes_remote; | ||||||
|  | 
 | ||||||
|  |     RefPtr<Core::Notifier> m_write_notifier; | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
| namespace Constants { | namespace Constants { | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Andreas Kling
						Andreas Kling