From bce16cdd6e767188e19425d16b60c02a0dc20b61 Mon Sep 17 00:00:00 2001 From: Bananymous Date: Wed, 19 Jun 2024 10:39:44 +0300 Subject: [PATCH] Kernel: Fix how socket closing works Sockets are now closed only when they are not referenced any more. This allows child process to close socket and still keep it open for the parent. --- kernel/include/kernel/FS/Inode.h | 4 - kernel/include/kernel/Networking/TCPSocket.h | 3 +- kernel/include/kernel/Networking/UDPSocket.h | 3 +- .../include/kernel/Networking/UNIX/Socket.h | 4 +- kernel/kernel/FS/Inode.cpp | 6 -- kernel/kernel/Networking/TCPSocket.cpp | 79 +++++++++++-------- kernel/kernel/Networking/UDPSocket.cpp | 2 +- kernel/kernel/Networking/UNIX/Socket.cpp | 2 +- kernel/kernel/OpenFileDescriptorSet.cpp | 2 - 9 files changed, 50 insertions(+), 55 deletions(-) diff --git a/kernel/include/kernel/FS/Inode.h b/kernel/include/kernel/FS/Inode.h index b9647349..c79615d8 100644 --- a/kernel/include/kernel/FS/Inode.h +++ b/kernel/include/kernel/FS/Inode.h @@ -86,8 +86,6 @@ namespace Kernel virtual bool is_pipe() const { return false; } virtual bool is_tty() const { return false; } - void on_close(); - // Directory API BAN::ErrorOr> find_inode(BAN::StringView); BAN::ErrorOr list_next_inodes(off_t, struct dirent* list, size_t list_size); @@ -122,8 +120,6 @@ namespace Kernel BAN::ErrorOr ioctl(int request, void* arg); protected: - virtual void on_close_impl() {} - // Directory API virtual BAN::ErrorOr> find_inode_impl(BAN::StringView) { return BAN::Error::from_errno(ENOTSUP); } virtual BAN::ErrorOr list_next_inodes_impl(off_t, struct dirent*, size_t) { return BAN::Error::from_errno(ENOTSUP); } diff --git a/kernel/include/kernel/Networking/TCPSocket.h b/kernel/include/kernel/Networking/TCPSocket.h index 839142a7..a031d99d 100644 --- a/kernel/include/kernel/Networking/TCPSocket.h +++ b/kernel/include/kernel/Networking/TCPSocket.h @@ -49,8 +49,6 @@ namespace Kernel virtual void add_protocol_header(BAN::ByteSpan packet, uint16_t dst_port, PseudoHeader) override; protected: - virtual void on_close_impl() override; - virtual BAN::ErrorOr connect_impl(const sockaddr*, socklen_t) override; virtual void receive_packet(BAN::ConstByteSpan, const sockaddr_storage& sender) override; @@ -111,6 +109,7 @@ namespace Kernel TCPSocket(NetworkLayer&, ino_t, const TmpInodeInfo&); void process_task(); + void start_close_sequence(); void set_connection_as_closed(); private: diff --git a/kernel/include/kernel/Networking/UDPSocket.h b/kernel/include/kernel/Networking/UDPSocket.h index ae1b48a1..1ba27f61 100644 --- a/kernel/include/kernel/Networking/UDPSocket.h +++ b/kernel/include/kernel/Networking/UDPSocket.h @@ -33,8 +33,6 @@ namespace Kernel protected: virtual void receive_packet(BAN::ConstByteSpan, const sockaddr_storage& sender) override; - virtual void on_close_impl() override; - virtual BAN::ErrorOr bind_impl(const sockaddr* address, socklen_t address_len) override; virtual BAN::ErrorOr sendto_impl(BAN::ConstByteSpan message, const sockaddr* address, socklen_t address_len) override; virtual BAN::ErrorOr recvfrom_impl(BAN::ByteSpan buffer, sockaddr* address, socklen_t* address_len) override; @@ -45,6 +43,7 @@ namespace Kernel private: UDPSocket(NetworkLayer&, ino_t, const TmpInodeInfo&); + ~UDPSocket(); struct PacketInfo { diff --git a/kernel/include/kernel/Networking/UNIX/Socket.h b/kernel/include/kernel/Networking/UNIX/Socket.h index 3766c34d..fb995acf 100644 --- a/kernel/include/kernel/Networking/UNIX/Socket.h +++ b/kernel/include/kernel/Networking/UNIX/Socket.h @@ -18,8 +18,6 @@ namespace Kernel static BAN::ErrorOr> create(SocketType, ino_t, const TmpInodeInfo&); protected: - virtual void on_close_impl() override; - virtual BAN::ErrorOr accept_impl(sockaddr*, socklen_t*) override; virtual BAN::ErrorOr connect_impl(const sockaddr*, socklen_t) override; virtual BAN::ErrorOr listen_impl(int) override; @@ -33,7 +31,7 @@ namespace Kernel private: UnixDomainSocket(SocketType, ino_t, const TmpInodeInfo&); - ~UnixDomainSocket() { on_close_impl(); } + ~UnixDomainSocket(); BAN::ErrorOr add_packet(BAN::ConstByteSpan); diff --git a/kernel/kernel/FS/Inode.cpp b/kernel/kernel/FS/Inode.cpp index a8a8a620..30e45b65 100644 --- a/kernel/kernel/FS/Inode.cpp +++ b/kernel/kernel/FS/Inode.cpp @@ -56,12 +56,6 @@ namespace Kernel return true; } - void Inode::on_close() - { - LockGuard _(m_mutex); - on_close_impl(); - } - BAN::ErrorOr> Inode::find_inode(BAN::StringView name) { LockGuard _(m_mutex); diff --git a/kernel/kernel/Networking/TCPSocket.cpp b/kernel/kernel/Networking/TCPSocket.cpp index a23bd4a3..21253f5a 100644 --- a/kernel/kernel/Networking/TCPSocket.cpp +++ b/kernel/kernel/Networking/TCPSocket.cpp @@ -63,39 +63,6 @@ namespace Kernel { ASSERT(!is_bound()); ASSERT(m_process == nullptr); - dprintln_if(DEBUG_TCP, "socket destroyed"); - } - - void TCPSocket::on_close_impl() - { - LockGuard _(m_mutex); - - if (!is_bound()) - return; - - switch (m_state) - { - case State::Established: - break; - case State::SynSent: - set_connection_as_closed(); - // fall through - case State::SynReceived: - case State::FinWait1: - case State::FinWait2: - case State::CloseWait: - case State::Closing: - case State::TimeWait: - case State::LastAck: - return; - case State::Closed: ASSERT_NOT_REACHED(); - case State::Listen: ASSERT_NOT_REACHED(); - } - - m_state = State::FinWait1; - m_should_ack = true; - - dprintln_if(DEBUG_TCP, "Initiated close"); } BAN::ErrorOr TCPSocket::connect_impl(const sockaddr* address, socklen_t address_len) @@ -407,6 +374,38 @@ namespace Kernel m_semaphore.unblock(); } + + void TCPSocket::start_close_sequence() + { + LockGuard _(m_mutex); + + if (!is_bound()) + return; + + switch (m_state) + { + case State::Established: + break; + case State::SynSent: + set_connection_as_closed(); + return; + case State::SynReceived: + case State::FinWait1: + case State::FinWait2: + case State::CloseWait: + case State::Closing: + case State::TimeWait: + case State::LastAck: + return; + case State::Closed: ASSERT_NOT_REACHED(); + case State::Listen: ASSERT_NOT_REACHED(); + } + + m_state = State::FinWait1; + m_should_ack = true; + + dprintln_if(DEBUG_TCP, "Initiated close"); + } void TCPSocket::set_connection_as_closed() { @@ -427,13 +426,25 @@ namespace Kernel static constexpr uint32_t retransmit_timeout_ms = 1000; BAN::RefPtr keep_alive = this; + bool started_close_sequence = false; while (m_process) { - uint64_t current_ms = SystemTimer::get().ms_since_boot(); + const uint64_t current_ms = SystemTimer::get().ms_since_boot(); if (m_state == State::TimeWait && current_ms >= m_time_wait_start_ms + 30'000) + { set_connection_as_closed(); + continue; + } + + // This is the last instance + if (!started_close_sequence && ref_count() == 1) + { + start_close_sequence(); + started_close_sequence = true; + continue; + } { LockGuard _(m_mutex); diff --git a/kernel/kernel/Networking/UDPSocket.cpp b/kernel/kernel/Networking/UDPSocket.cpp index 6345b694..def27747 100644 --- a/kernel/kernel/Networking/UDPSocket.cpp +++ b/kernel/kernel/Networking/UDPSocket.cpp @@ -23,7 +23,7 @@ namespace Kernel : NetworkSocket(network_layer, ino, inode_info) { } - void UDPSocket::on_close_impl() + UDPSocket::~UDPSocket() { if (is_bound()) m_network_layer.unbind_socket(this, m_port); diff --git a/kernel/kernel/Networking/UNIX/Socket.cpp b/kernel/kernel/Networking/UNIX/Socket.cpp index 7232f608..cef3c505 100644 --- a/kernel/kernel/Networking/UNIX/Socket.cpp +++ b/kernel/kernel/Networking/UNIX/Socket.cpp @@ -47,7 +47,7 @@ namespace Kernel } } - void UnixDomainSocket::on_close_impl() + UnixDomainSocket::~UnixDomainSocket() { if (is_bound() && !is_bound_to_unused()) { diff --git a/kernel/kernel/OpenFileDescriptorSet.cpp b/kernel/kernel/OpenFileDescriptorSet.cpp index 58fa666c..e5b4bbc7 100644 --- a/kernel/kernel/OpenFileDescriptorSet.cpp +++ b/kernel/kernel/OpenFileDescriptorSet.cpp @@ -317,8 +317,6 @@ namespace Kernel if (m_open_files[fd]->flags & O_WRONLY && m_open_files[fd]->inode->is_pipe()) ((Pipe*)m_open_files[fd]->inode.ptr())->close_writing(); - m_open_files[fd]->inode->on_close(); - m_open_files[fd].clear(); return {};