From ae5d7f542c256f3f1b4c5003d666e8ef850516e9 Mon Sep 17 00:00:00 2001 From: Peter Elliott Date: Sun, 12 Feb 2023 00:49:15 -0700 Subject: [PATCH] Kernel: Change polarity of weak ownership between Inode and LocalSocket There was a bug in which bound Inodes would lose all their references (because localsocket does not reference them), and they would be deallocated, and clients would get ECONNREFUSED as a result. now LocalSocket has a strong reference to inode so that the inode will live as long as the socket, and Inode has a weak reference to the socket, because if the socket stops being referenced anywhere it should not be bound. This still prevents the reference loop that 220b7dd77905b7d573ded093cf88d2dc51f57c69 was trying to fix. --- Kernel/FileSystem/Inode.cpp | 2 +- Kernel/FileSystem/Inode.h | 2 +- Kernel/Net/LocalSocket.cpp | 5 ++--- Kernel/Net/LocalSocket.h | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Kernel/FileSystem/Inode.cpp b/Kernel/FileSystem/Inode.cpp index e6b8f1ef14..1eea0c0b82 100644 --- a/Kernel/FileSystem/Inode.cpp +++ b/Kernel/FileSystem/Inode.cpp @@ -143,7 +143,7 @@ ErrorOr Inode::set_shared_vmobject(Memory::SharedInodeVMObject& vmobject) LockRefPtr Inode::bound_socket() const { - return m_bound_socket; + return m_bound_socket.strong_ref(); } bool Inode::bind_socket(LocalSocket& socket) diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h index c954de927a..8a327ff266 100644 --- a/Kernel/FileSystem/Inode.h +++ b/Kernel/FileSystem/Inode.h @@ -131,7 +131,7 @@ private: FileSystem& m_file_system; InodeIndex m_index { 0 }; LockWeakPtr m_shared_vmobject; - LockRefPtr m_bound_socket; + LockWeakPtr m_bound_socket; SpinlockProtected, LockRank::None> m_watchers {}; bool m_metadata_dirty { false }; LockRefPtr m_fifo; diff --git a/Kernel/Net/LocalSocket.cpp b/Kernel/Net/LocalSocket.cpp index bc4afc01e3..4b55e8bea2 100644 --- a/Kernel/Net/LocalSocket.cpp +++ b/Kernel/Net/LocalSocket.cpp @@ -260,9 +260,8 @@ void LocalSocket::detach(OpenFileDescription& description) m_accept_side_fd_open = false; if (m_bound) { - auto inode = m_inode.strong_ref(); - if (inode) - inode->unbind_socket(); + if (m_inode) + m_inode->unbind_socket(); } } diff --git a/Kernel/Net/LocalSocket.h b/Kernel/Net/LocalSocket.h index 91722d72cd..5416bd5373 100644 --- a/Kernel/Net/LocalSocket.h +++ b/Kernel/Net/LocalSocket.h @@ -73,7 +73,7 @@ private: ErrorOr try_set_path(StringView); // The inode this socket is bound to. - LockWeakPtr m_inode; + LockRefPtr m_inode; UserID m_prebind_uid { 0 }; GroupID m_prebind_gid { 0 };