Browse Source

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.
Peter Elliott 2 years ago
parent
commit
ae5d7f542c

+ 1 - 1
Kernel/FileSystem/Inode.cpp

@@ -143,7 +143,7 @@ ErrorOr<void> Inode::set_shared_vmobject(Memory::SharedInodeVMObject& vmobject)
 
 
 LockRefPtr<LocalSocket> Inode::bound_socket() const
 LockRefPtr<LocalSocket> Inode::bound_socket() const
 {
 {
-    return m_bound_socket;
+    return m_bound_socket.strong_ref();
 }
 }
 
 
 bool Inode::bind_socket(LocalSocket& socket)
 bool Inode::bind_socket(LocalSocket& socket)

+ 1 - 1
Kernel/FileSystem/Inode.h

@@ -131,7 +131,7 @@ private:
     FileSystem& m_file_system;
     FileSystem& m_file_system;
     InodeIndex m_index { 0 };
     InodeIndex m_index { 0 };
     LockWeakPtr<Memory::SharedInodeVMObject> m_shared_vmobject;
     LockWeakPtr<Memory::SharedInodeVMObject> m_shared_vmobject;
-    LockRefPtr<LocalSocket> m_bound_socket;
+    LockWeakPtr<LocalSocket> m_bound_socket;
     SpinlockProtected<HashTable<InodeWatcher*>, LockRank::None> m_watchers {};
     SpinlockProtected<HashTable<InodeWatcher*>, LockRank::None> m_watchers {};
     bool m_metadata_dirty { false };
     bool m_metadata_dirty { false };
     LockRefPtr<FIFO> m_fifo;
     LockRefPtr<FIFO> m_fifo;

+ 2 - 3
Kernel/Net/LocalSocket.cpp

@@ -260,9 +260,8 @@ void LocalSocket::detach(OpenFileDescription& description)
         m_accept_side_fd_open = false;
         m_accept_side_fd_open = false;
 
 
         if (m_bound) {
         if (m_bound) {
-            auto inode = m_inode.strong_ref();
-            if (inode)
-                inode->unbind_socket();
+            if (m_inode)
+                m_inode->unbind_socket();
         }
         }
     }
     }
 
 

+ 1 - 1
Kernel/Net/LocalSocket.h

@@ -73,7 +73,7 @@ private:
     ErrorOr<void> try_set_path(StringView);
     ErrorOr<void> try_set_path(StringView);
 
 
     // The inode this socket is bound to.
     // The inode this socket is bound to.
-    LockWeakPtr<Inode> m_inode;
+    LockRefPtr<Inode> m_inode;
 
 
     UserID m_prebind_uid { 0 };
     UserID m_prebind_uid { 0 };
     GroupID m_prebind_gid { 0 };
     GroupID m_prebind_gid { 0 };