From 75f61fe3d9055b4e65deecc796401ea2f9e827b2 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 29 Sep 2020 16:26:13 -0600 Subject: [PATCH] AK: Make RefPtr, NonnullRefPtr, WeakPtr thread safe This makes most operations thread safe, especially so that they can safely be used in the Kernel. This includes obtaining a strong reference from a weak reference, which now requires an explicit call to WeakPtr::strong_ref(). Another major change is that Weakable::make_weak_ref() may require the explicit target type. Previously we used reinterpret_cast in WeakPtr, assuming that it can be properly converted. But WeakPtr does not necessarily have the knowledge to be able to do this. Instead, we now ask the class itself to deliver a WeakPtr to the type that we want. Also, WeakLink is no longer specific to a target type. The reason for this is that we want to be able to safely convert e.g. WeakPtr to WeakPtr, and before this we just reinterpret_cast the internal WeakLink to WeakLink, which is a bold assumption that it would actually produce the correct code. Instead, WeakLink now operates on just a raw pointer and we only make those constructors/operators available if we can verify that it can be safely cast. In order to guarantee thread safety, we now use the least significant bit in the pointer for locking purposes. This also means that only properly aligned pointers can be used. --- AK/NonnullRefPtr.h | 198 ++++++++--- AK/RefCounted.h | 8 +- AK/RefPtr.h | 329 +++++++++++++----- AK/StdLibExtras.h | 3 +- AK/Tests/TestRefPtr.cpp | 39 +++ AK/Tests/TestWeakPtr.cpp | 23 +- AK/WeakPtr.h | 199 +++++++++-- AK/Weakable.h | 58 ++- Applications/PixelPaint/Tool.cpp | 2 +- Applications/Spreadsheet/Spreadsheet.cpp | 4 +- Applications/Spreadsheet/Spreadsheet.h | 2 +- DevTools/HackStudio/Editor.cpp | 2 +- Kernel/FileSystem/DevPtsFS.cpp | 6 +- Kernel/FileSystem/Inode.cpp | 25 +- Kernel/FileSystem/Inode.h | 5 +- Kernel/FileSystem/InodeWatcher.cpp | 12 +- Kernel/Net/TCPSocket.cpp | 2 +- Kernel/Net/TCPSocket.h | 2 +- Kernel/Process.cpp | 6 +- Kernel/Scheduler.cpp | 5 +- Kernel/SharedBuffer.cpp | 15 +- Kernel/Syscalls/fork.cpp | 4 +- Kernel/TTY/TTY.cpp | 8 +- Kernel/TTY/TTY.h | 7 +- Kernel/Thread.cpp | 2 +- Kernel/VM/SharedInodeVMObject.cpp | 2 +- Libraries/LibCore/Event.cpp | 28 ++ Libraries/LibCore/Event.h | 8 +- Libraries/LibCore/EventLoop.cpp | 33 +- Libraries/LibGUI/Application.cpp | 2 +- Libraries/LibGUI/Button.cpp | 2 +- Libraries/LibGUI/Layout.cpp | 6 +- Libraries/LibGUI/Menu.cpp | 2 +- Libraries/LibGUI/Splitter.cpp | 4 +- Libraries/LibGUI/SyntaxHighlighter.cpp | 2 +- Libraries/LibGUI/Widget.cpp | 5 +- Libraries/LibGUI/Window.cpp | 10 +- Libraries/LibProtocol/Download.cpp | 2 +- Libraries/LibWeb/CSS/StyleValue.cpp | 2 +- Libraries/LibWeb/DOM/Document.cpp | 7 +- .../LibWeb/HTML/CanvasRenderingContext2D.cpp | 2 +- Libraries/LibWeb/HTML/HTMLScriptElement.cpp | 6 +- Libraries/LibWeb/Page/Frame.cpp | 2 +- Services/AudioServer/Mixer.cpp | 2 +- Services/WindowServer/AppletManager.cpp | 2 +- Services/WindowServer/Menu.h | 2 +- Services/WindowServer/MenuManager.cpp | 10 +- Services/WindowServer/Window.cpp | 6 +- Services/WindowServer/WindowManager.cpp | 26 +- Services/WindowServer/WindowSwitcher.cpp | 2 +- 50 files changed, 819 insertions(+), 322 deletions(-) diff --git a/AK/NonnullRefPtr.h b/AK/NonnullRefPtr.h index fab7f227d2e..04e55a0985d 100644 --- a/AK/NonnullRefPtr.h +++ b/AK/NonnullRefPtr.h @@ -27,9 +27,13 @@ #pragma once #include +#include #include #include #include +#ifdef KERNEL +# include +#endif namespace AK { @@ -54,55 +58,66 @@ ALWAYS_INLINE void unref_if_not_null(T* ptr) template class NonnullRefPtr { + template + friend class RefPtr; + template + friend class NonnullRefPtr; + template + friend class WeakPtr; + public: typedef T ElementType; enum AdoptTag { Adopt }; ALWAYS_INLINE NonnullRefPtr(const T& object) - : m_ptr(const_cast(&object)) + : m_bits((FlatPtr)&object) { - m_ptr->ref(); + ASSERT(!(m_bits & 1)); + const_cast(object).ref(); } template ALWAYS_INLINE NonnullRefPtr(const U& object) - : m_ptr(&const_cast(object)) + : m_bits((FlatPtr) static_cast(&object)) { - m_ptr->ref(); + ASSERT(!(m_bits & 1)); + const_cast(static_cast(object)).ref(); } ALWAYS_INLINE NonnullRefPtr(AdoptTag, T& object) - : m_ptr(&object) + : m_bits((FlatPtr)&object) { + ASSERT(!(m_bits & 1)); } ALWAYS_INLINE NonnullRefPtr(NonnullRefPtr&& other) - : m_ptr(&other.leak_ref()) + : m_bits((FlatPtr)&other.leak_ref()) { + ASSERT(!(m_bits & 1)); } template ALWAYS_INLINE NonnullRefPtr(NonnullRefPtr&& other) - : m_ptr(&other.leak_ref()) + : m_bits((FlatPtr)&other.leak_ref()) { + ASSERT(!(m_bits & 1)); } ALWAYS_INLINE NonnullRefPtr(const NonnullRefPtr& other) - : m_ptr(const_cast(other.ptr())) + : m_bits((FlatPtr)other.add_ref()) { - m_ptr->ref(); + ASSERT(!(m_bits & 1)); } template ALWAYS_INLINE NonnullRefPtr(const NonnullRefPtr& other) - : m_ptr(const_cast(other.ptr())) + : m_bits((FlatPtr)other.add_ref()) { - m_ptr->ref(); + ASSERT(!(m_bits & 1)); } ALWAYS_INLINE ~NonnullRefPtr() { - unref_if_not_null(m_ptr); - m_ptr = nullptr; + assign(nullptr); #ifdef SANITIZE_PTRS if constexpr (sizeof(T*) == 8) - m_ptr = (T*)(0xb0b0b0b0b0b0b0b0); + m_bits.store(0xb0b0b0b0b0b0b0b0, AK::MemoryOrder::memory_order_relaxed); else - m_ptr = (T*)(0xb0b0b0b0); + m_bits.store(0xb0b0b0b0, AK::MemoryOrder::memory_order_relaxed); #endif } @@ -120,100 +135,89 @@ public: NonnullRefPtr& operator=(const NonnullRefPtr& other) { - NonnullRefPtr ptr(other); - swap(ptr); + if (this != &other) + assign(other.add_ref()); return *this; } template NonnullRefPtr& operator=(const NonnullRefPtr& other) { - NonnullRefPtr ptr(other); - swap(ptr); + assign(other.add_ref()); return *this; } ALWAYS_INLINE NonnullRefPtr& operator=(NonnullRefPtr&& other) { - NonnullRefPtr ptr(move(other)); - swap(ptr); + if (this != &other) + assign(&other.leak_ref()); return *this; } template NonnullRefPtr& operator=(NonnullRefPtr&& other) { - NonnullRefPtr ptr(move(other)); - swap(ptr); + assign(&other.leak_ref()); return *this; } NonnullRefPtr& operator=(const T& object) { - NonnullRefPtr ptr(object); - swap(ptr); + const_cast(object).ref(); + assign(const_cast(&object)); return *this; } [[nodiscard]] ALWAYS_INLINE T& leak_ref() { - ASSERT(m_ptr); - return *exchange(m_ptr, nullptr); + T* ptr = exchange(nullptr); + ASSERT(ptr); + return *ptr; } ALWAYS_INLINE T* ptr() { - ASSERT(m_ptr); - return m_ptr; + return as_nonnull_ptr(); } ALWAYS_INLINE const T* ptr() const { - ASSERT(m_ptr); - return m_ptr; + return as_nonnull_ptr(); } ALWAYS_INLINE T* operator->() { - ASSERT(m_ptr); - return m_ptr; + return as_nonnull_ptr(); } ALWAYS_INLINE const T* operator->() const { - ASSERT(m_ptr); - return m_ptr; + return as_nonnull_ptr(); } ALWAYS_INLINE T& operator*() { - ASSERT(m_ptr); - return *m_ptr; + return *as_nonnull_ptr(); } ALWAYS_INLINE const T& operator*() const { - ASSERT(m_ptr); - return *m_ptr; + return *as_nonnull_ptr(); } ALWAYS_INLINE operator T*() { - ASSERT(m_ptr); - return m_ptr; + return as_nonnull_ptr(); } ALWAYS_INLINE operator const T*() const { - ASSERT(m_ptr); - return m_ptr; + return as_nonnull_ptr(); } ALWAYS_INLINE operator T&() { - ASSERT(m_ptr); - return *m_ptr; + return *as_nonnull_ptr(); } ALWAYS_INLINE operator const T&() const { - ASSERT(m_ptr); - return *m_ptr; + return *as_nonnull_ptr(); } operator bool() const = delete; @@ -221,19 +225,113 @@ public: void swap(NonnullRefPtr& other) { - ::swap(m_ptr, other.m_ptr); + if (this == &other) + return; + + // NOTE: swap is not atomic! + T* other_ptr = other.exchange(nullptr); + T* ptr = exchange(other_ptr); + other.exchange(ptr); } template void swap(NonnullRefPtr& other) { - ::swap(m_ptr, other.m_ptr); + // NOTE: swap is not atomic! + U* other_ptr = other.exchange(nullptr); + T* ptr = exchange(other_ptr); + other.exchange(ptr); } private: NonnullRefPtr() = delete; - T* m_ptr { nullptr }; + ALWAYS_INLINE T* as_ptr() const + { + return (T*)(m_bits.load(AK::MemoryOrder::memory_order_relaxed) & ~(FlatPtr)1); + } + + ALWAYS_INLINE T* as_nonnull_ptr() const + { + T* ptr = (T*)(m_bits.load(AK::MemoryOrder::memory_order_relaxed) & ~(FlatPtr)1); + ASSERT(ptr); + return ptr; + } + + template + void do_while_locked(F f) const + { +#ifdef KERNEL + // We don't want to be pre-empted while we have the lock bit set + Kernel::ScopedCritical critical; +#endif + FlatPtr bits; + for (;;) { + bits = m_bits.fetch_or(1, AK::MemoryOrder::memory_order_acq_rel); + if (!(bits & 1)) + break; +#ifdef KERNEL + Kernel::Processor::wait_check(); +#endif + } + ASSERT(!(bits & 1)); + f((T*)bits); + m_bits.store(bits, AK::MemoryOrder::memory_order_release); + } + + ALWAYS_INLINE void assign(T* new_ptr) + { + T* prev_ptr = exchange(new_ptr); + unref_if_not_null(prev_ptr); + } + + ALWAYS_INLINE T* exchange(T* new_ptr) + { + ASSERT(!((FlatPtr)new_ptr & 1)); +#ifdef KERNEL + // We don't want to be pre-empted while we have the lock bit set + Kernel::ScopedCritical critical; +#endif + // Only exchange while not locked + FlatPtr expected = m_bits.load(AK::MemoryOrder::memory_order_relaxed); + for (;;) { + expected &= ~(FlatPtr)1; // only if lock bit is not set + if (m_bits.compare_exchange_strong(expected, (FlatPtr)new_ptr, AK::MemoryOrder::memory_order_acq_rel)) + break; +#ifdef KERNEL + Kernel::Processor::wait_check(); +#endif + } + ASSERT(!(expected & 1)); + return (T*)expected; + } + + T* add_ref() const + { +#ifdef KERNEL + // We don't want to be pre-empted while we have the lock bit set + Kernel::ScopedCritical critical; +#endif + // Lock the pointer + FlatPtr expected = m_bits.load(AK::MemoryOrder::memory_order_relaxed); + for (;;) { + expected &= ~(FlatPtr)1; // only if lock bit is not set + if (m_bits.compare_exchange_strong(expected, expected | 1, AK::MemoryOrder::memory_order_acq_rel)) + break; +#ifdef KERNEL + Kernel::Processor::wait_check(); +#endif + } + + // Add a reference now that we locked the pointer + ref_if_not_null((T*)expected); + + // Unlock the pointer again + m_bits.store(expected, AK::MemoryOrder::memory_order_release); + return (T*)expected; + } + + mutable Atomic m_bits { 0 }; }; template diff --git a/AK/RefCounted.h b/AK/RefCounted.h index 86b6d76aab8..fac57602705 100644 --- a/AK/RefCounted.h +++ b/AK/RefCounted.h @@ -69,26 +69,26 @@ public: ALWAYS_INLINE void ref() const { - auto old_ref_count = m_ref_count++; + auto old_ref_count = m_ref_count.fetch_add(1, AK::MemoryOrder::memory_order_relaxed); ASSERT(old_ref_count > 0); ASSERT(!Checked::addition_would_overflow(old_ref_count, 1)); } ALWAYS_INLINE RefCountType ref_count() const { - return m_ref_count; + return m_ref_count.load(AK::MemoryOrder::memory_order_relaxed); } protected: RefCountedBase() { } ALWAYS_INLINE ~RefCountedBase() { - ASSERT(m_ref_count == 0); + ASSERT(m_ref_count.load(AK::MemoryOrder::memory_order_relaxed) == 0); } ALWAYS_INLINE RefCountType deref_base() const { - auto old_ref_count = m_ref_count--; + auto old_ref_count = m_ref_count.fetch_sub(1, AK::MemoryOrder::memory_order_acq_rel); ASSERT(old_ref_count > 0); return old_ref_count - 1; } diff --git a/AK/RefPtr.h b/AK/RefPtr.h index 2ce4ed81614..aef74bd6da5 100644 --- a/AK/RefPtr.h +++ b/AK/RefPtr.h @@ -26,11 +26,15 @@ #pragma once +#include #include #include #include #include #include +#ifdef KERNEL +# include +#endif namespace AK { @@ -39,19 +43,87 @@ class OwnPtr; template struct RefPtrTraits { - static T* as_ptr(FlatPtr bits) + ALWAYS_INLINE static T* as_ptr(FlatPtr bits) { - return (T*)bits; + return (T*)(bits & ~(FlatPtr)1); } - static FlatPtr as_bits(T* ptr) + ALWAYS_INLINE static FlatPtr as_bits(T* ptr) { + ASSERT(!((FlatPtr)ptr & 1)); return (FlatPtr)ptr; } - static bool is_null(FlatPtr bits) + template + ALWAYS_INLINE static FlatPtr convert_from(FlatPtr bits) { - return !bits; + if (PtrTraits::is_null(bits)) + return default_null_value; + return as_bits(PtrTraits::as_ptr(bits)); + } + + ALWAYS_INLINE static bool is_null(FlatPtr bits) + { + return !(bits & ~(FlatPtr)1); + } + + ALWAYS_INLINE static FlatPtr exchange(Atomic& atomic_var, FlatPtr new_value) + { + // Only exchange when lock is not held + ASSERT(!(new_value & 1)); + FlatPtr expected = atomic_var.load(AK::MemoryOrder::memory_order_relaxed); + for (;;) { + expected &= ~(FlatPtr)1; // only if lock bit is not set + if (atomic_var.compare_exchange_strong(expected, new_value, AK::MemoryOrder::memory_order_acq_rel)) + break; +#ifdef KERNEL + Kernel::Processor::wait_check(); +#endif + } + return expected; + } + + ALWAYS_INLINE static bool exchange_if_null(Atomic& atomic_var, FlatPtr new_value) + { + // Only exchange when lock is not held + ASSERT(!(new_value & 1)); + for (;;) { + FlatPtr expected = default_null_value; // only if lock bit is not set + if (atomic_var.compare_exchange_strong(expected, new_value, AK::MemoryOrder::memory_order_acq_rel)) + break; + if (!is_null(expected)) + return false; +#ifdef KERNEL + Kernel::Processor::wait_check(); +#endif + } + return true; + } + + ALWAYS_INLINE static FlatPtr lock(Atomic& atomic_var) + { + // This sets the lock bit atomically, preventing further modifications. + // This is important when e.g. copying a RefPtr where the source + // might be released and freed too quickly. This allows us + // to temporarily lock the pointer so we can add a reference, then + // unlock it + FlatPtr bits; + for (;;) { + bits = atomic_var.fetch_or(1, AK::MemoryOrder::memory_order_acq_rel); + if (!(bits & 1)) + break; +#ifdef KERNEL + Kernel::Processor::wait_check(); +#endif + } + ASSERT(!(bits & 1)); + return bits; + } + + ALWAYS_INLINE static void unlock(Atomic& atomic_var, FlatPtr new_value) + { + ASSERT(!(new_value & 1)); + atomic_var.store(new_value, AK::MemoryOrder::memory_order_release); } static constexpr FlatPtr default_null_value = 0; @@ -63,6 +135,9 @@ template class RefPtr { template friend class RefPtr; + template + friend class WeakPtr; + public: enum AdoptTag { Adopt @@ -79,62 +154,55 @@ public: { T* ptr = const_cast(&object); ASSERT(ptr); - ASSERT(!ptr == PtrTraits::is_null(m_bits)); + ASSERT(!is_null()); ptr->ref(); } RefPtr(AdoptTag, T& object) : m_bits(PtrTraits::as_bits(&object)) { - ASSERT(&object); - ASSERT(!PtrTraits::is_null(m_bits)); + ASSERT(!is_null()); } RefPtr(RefPtr&& other) : m_bits(other.leak_ref_raw()) { } ALWAYS_INLINE RefPtr(const NonnullRefPtr& other) - : m_bits(PtrTraits::as_bits(const_cast(other.ptr()))) + : m_bits(PtrTraits::as_bits(const_cast(other.add_ref()))) { - ASSERT(!PtrTraits::is_null(m_bits)); - PtrTraits::as_ptr(m_bits)->ref(); } template ALWAYS_INLINE RefPtr(const NonnullRefPtr& other) - : m_bits(PtrTraits::as_bits(const_cast(other.ptr()))) + : m_bits(PtrTraits::as_bits(const_cast(other.add_ref()))) { - ASSERT(!PtrTraits::is_null(m_bits)); - PtrTraits::as_ptr(m_bits)->ref(); } template ALWAYS_INLINE RefPtr(NonnullRefPtr&& other) : m_bits(PtrTraits::as_bits(&other.leak_ref())) { - ASSERT(!PtrTraits::is_null(m_bits)); + ASSERT(!is_null()); } template> RefPtr(RefPtr&& other) - : m_bits(other.leak_ref_raw()) + : m_bits(PtrTraits::template convert_from(other.leak_ref_raw())) { } RefPtr(const RefPtr& other) - : m_bits(PtrTraits::as_bits(const_cast(other.ptr()))) + : m_bits(other.add_ref_raw()) { - ref_if_not_null(const_cast(other.ptr())); } template> RefPtr(const RefPtr& other) - : m_bits(PtrTraits::as_bits(const_cast(other.ptr()))) + : m_bits(other.add_ref_raw()) { - ref_if_not_null(const_cast(other.ptr())); } ALWAYS_INLINE ~RefPtr() { clear(); #ifdef SANITIZE_PTRS if constexpr (sizeof(T*) == 8) - m_bits = 0xe0e0e0e0e0e0e0e0; + m_bits.store(0xe0e0e0e0e0e0e0e0, AK::MemoryOrder::memory_order_relaxed); else - m_bits = 0xe0e0e0e0; + m_bits.store(0xe0e0e0e0, AK::MemoryOrder::memory_order_relaxed); #endif } RefPtr(std::nullptr_t) { } @@ -144,79 +212,85 @@ public: template RefPtr& operator=(const OwnPtr&) = delete; - template - void swap(RefPtr& other) + void swap(RefPtr& other) { - ::swap(m_bits, other.m_bits); + if (this == &other) + return; + + // NOTE: swap is not atomic! + FlatPtr other_bits = PtrTraits::exchange(other.m_bits, PtrTraits::default_null_value); + FlatPtr bits = PtrTraits::exchange(m_bits, other_bits); + PtrTraits::exchange(other.m_bits, bits); + } + + template> + void swap(RefPtr& other) + { + // NOTE: swap is not atomic! + FlatPtr other_bits = P::exchange(other.m_bits, P::default_null_value); + FlatPtr bits = PtrTraits::exchange(m_bits, PtrTraits::template convert_from(other_bits)); + P::exchange(other.m_bits, P::template convert_from(bits)); } ALWAYS_INLINE RefPtr& operator=(RefPtr&& other) { - RefPtr tmp = move(other); - swap(tmp); + if (this != &other) + assign_raw(other.leak_ref_raw()); return *this; } - template - ALWAYS_INLINE RefPtr& operator=(RefPtr&& other) + template> + ALWAYS_INLINE RefPtr& operator=(RefPtr&& other) { - RefPtr tmp = move(other); - swap(tmp); + assign_raw(PtrTraits::template convert_from(other.leak_ref_raw())); return *this; } template ALWAYS_INLINE RefPtr& operator=(NonnullRefPtr&& other) { - RefPtr tmp = move(other); - swap(tmp); - ASSERT(!PtrTraits::is_null(m_bits)); + assign_raw(PtrTraits::as_bits(&other.leak_ref())); return *this; } ALWAYS_INLINE RefPtr& operator=(const NonnullRefPtr& other) { - RefPtr tmp = other; - swap(tmp); - ASSERT(!PtrTraits::is_null(m_bits)); + assign_raw(PtrTraits::as_bits(other.add_ref())); return *this; } template ALWAYS_INLINE RefPtr& operator=(const NonnullRefPtr& other) { - RefPtr tmp = other; - swap(tmp); - ASSERT(!PtrTraits::is_null(m_bits)); + assign_raw(PtrTraits::as_bits(other.add_ref())); return *this; } ALWAYS_INLINE RefPtr& operator=(const RefPtr& other) { - RefPtr tmp = other; - swap(tmp); + if (this != &other) + assign_raw(other.add_ref_raw()); return *this; } template ALWAYS_INLINE RefPtr& operator=(const RefPtr& other) { - RefPtr tmp = other; - swap(tmp); + assign_raw(other.add_ref_raw()); return *this; } ALWAYS_INLINE RefPtr& operator=(const T* ptr) { - RefPtr tmp = ptr; - swap(tmp); + ref_if_not_null(const_cast(ptr)); + assign_raw(PtrTraits::as_bits(const_cast(ptr))); return *this; } ALWAYS_INLINE RefPtr& operator=(const T& object) { - RefPtr tmp = object; - swap(tmp); + const_cast(object).ref(); + assign_raw(PtrTraits::as_bits(const_cast(&object))); return *this; } @@ -226,99 +300,166 @@ public: return *this; } - ALWAYS_INLINE void clear() + ALWAYS_INLINE bool assign_if_null(RefPtr&& other) { - unref_if_not_null(PtrTraits::as_ptr(m_bits)); - m_bits = PtrTraits::default_null_value; + if (this == &other) + return is_null(); + return PtrTraits::exchange_if_null(m_bits, other.leak_ref_raw()); } - bool operator!() const { return PtrTraits::is_null(m_bits); } + template> + ALWAYS_INLINE bool assign_if_null(RefPtr&& other) + { + if (this == &other) + return is_null(); + return PtrTraits::exchange_if_null(m_bits, PtrTraits::template convert_from(other.leak_ref_raw())); + } + + ALWAYS_INLINE void clear() + { + assign_raw(PtrTraits::default_null_value); + } + + bool operator!() const { return PtrTraits::is_null(m_bits.load(AK::MemoryOrder::memory_order_relaxed)); } [[nodiscard]] T* leak_ref() { - FlatPtr bits = exchange(m_bits, PtrTraits::default_null_value); - return !PtrTraits::is_null(bits) ? PtrTraits::as_ptr(bits) : nullptr; + FlatPtr bits = PtrTraits::exchange(m_bits, PtrTraits::default_null_value); + return PtrTraits::as_ptr(bits); } NonnullRefPtr release_nonnull() { - ASSERT(!PtrTraits::is_null(m_bits)); - return NonnullRefPtr(NonnullRefPtr::Adopt, *leak_ref()); + FlatPtr bits = PtrTraits::exchange(m_bits, PtrTraits::default_null_value); + ASSERT(!PtrTraits::is_null(bits)); + return NonnullRefPtr(NonnullRefPtr::Adopt, *PtrTraits::as_ptr(bits)); } - ALWAYS_INLINE T* ptr() { return !PtrTraits::is_null(m_bits) ? PtrTraits::as_ptr(m_bits) : nullptr; } - ALWAYS_INLINE const T* ptr() const { return !PtrTraits::is_null(m_bits) ? PtrTraits::as_ptr(m_bits) : nullptr; } + ALWAYS_INLINE T* ptr() { return as_ptr(); } + ALWAYS_INLINE const T* ptr() const { return as_ptr(); } ALWAYS_INLINE T* operator->() { - ASSERT(!PtrTraits::is_null(m_bits)); - return PtrTraits::as_ptr(m_bits); + return as_nonnull_ptr(); } ALWAYS_INLINE const T* operator->() const { - ASSERT(!PtrTraits::is_null(m_bits)); - return PtrTraits::as_ptr(m_bits); + return as_nonnull_ptr(); } ALWAYS_INLINE T& operator*() { - ASSERT(!PtrTraits::is_null(m_bits)); - return *PtrTraits::as_ptr(m_bits); + return *as_nonnull_ptr(); } ALWAYS_INLINE const T& operator*() const { - ASSERT(!PtrTraits::is_null(m_bits)); - return *PtrTraits::as_ptr(m_bits); + return *as_nonnull_ptr(); } - ALWAYS_INLINE operator const T*() const { return PtrTraits::as_ptr(m_bits); } - ALWAYS_INLINE operator T*() { return PtrTraits::as_ptr(m_bits); } + ALWAYS_INLINE operator const T*() const { return as_ptr(); } + ALWAYS_INLINE operator T*() { return as_ptr(); } - operator bool() { return !PtrTraits::is_null(m_bits); } + operator bool() { return !is_null(); } - bool operator==(std::nullptr_t) const { return PtrTraits::is_null(m_bits); } - bool operator!=(std::nullptr_t) const { return !PtrTraits::is_null(m_bits); } + bool operator==(std::nullptr_t) const { return is_null(); } + bool operator!=(std::nullptr_t) const { return !is_null(); } - bool operator==(const RefPtr& other) const { return m_bits == other.m_bits; } - bool operator!=(const RefPtr& other) const { return m_bits != other.m_bits; } + bool operator==(const RefPtr& other) const { return as_ptr() == other.as_ptr(); } + bool operator!=(const RefPtr& other) const { return as_ptr() != other.as_ptr(); } - bool operator==(RefPtr& other) { return m_bits == other.m_bits; } - bool operator!=(RefPtr& other) { return m_bits != other.m_bits; } + bool operator==(RefPtr& other) { return as_ptr() == other.as_ptr(); } + bool operator!=(RefPtr& other) { return as_ptr() != other.as_ptr(); } - bool operator==(const T* other) const { return PtrTraits::as_ptr(m_bits) == other; } - bool operator!=(const T* other) const { return PtrTraits::as_ptr(m_bits) != other; } + bool operator==(const T* other) const { return as_ptr() == other; } + bool operator!=(const T* other) const { return as_ptr() != other; } - bool operator==(T* other) { return PtrTraits::as_ptr(m_bits) == other; } - bool operator!=(T* other) { return PtrTraits::as_ptr(m_bits) != other; } + bool operator==(T* other) { return as_ptr() == other; } + bool operator!=(T* other) { return as_ptr() != other; } + + bool is_null() const { return PtrTraits::is_null(m_bits.load(AK::MemoryOrder::memory_order_relaxed)); } - bool is_null() const { return PtrTraits::is_null(m_bits); } - template::value && !IsNullPointer::value>::Type* = nullptr> typename PtrTraits::NullType null_value() const { // make sure we are holding a null value - ASSERT(PtrTraits::is_null(m_bits)); - return PtrTraits::to_null_value(m_bits); + FlatPtr bits = m_bits.load(AK::MemoryOrder::memory_order_relaxed); + ASSERT(PtrTraits::is_null(bits)); + return PtrTraits::to_null_value(bits); } template::value && !IsNullPointer::value>::Type* = nullptr> void set_null_value(typename PtrTraits::NullType value) { - // make sure that new null value would be interpreted as a null value - FlatPtr bits = PtrTraits::from_null_value(value); - ASSERT(PtrTraits::is_null(bits)); - clear(); - m_bits = bits; + // make sure that new null value would be interpreted as a null value + FlatPtr bits = PtrTraits::from_null_value(value); + ASSERT(PtrTraits::is_null(bits)); + assign_raw(bits); } private: - [[nodiscard]] FlatPtr leak_ref_raw() + template + void do_while_locked(F f) const { - return exchange(m_bits, PtrTraits::default_null_value); +#ifdef KERNEL + // We don't want to be pre-empted while we have the lock bit set + Kernel::ScopedCritical critical; +#endif + FlatPtr bits = PtrTraits::lock(m_bits); + T* ptr = PtrTraits::as_ptr(bits); + f(ptr); + PtrTraits::unlock(m_bits, bits); } - FlatPtr m_bits { PtrTraits::default_null_value }; + [[nodiscard]] ALWAYS_INLINE FlatPtr leak_ref_raw() + { + return PtrTraits::exchange(m_bits, PtrTraits::default_null_value); + } + + [[nodiscard]] ALWAYS_INLINE FlatPtr add_ref_raw() const + { +#ifdef KERNEL + // We don't want to be pre-empted while we have the lock bit set + Kernel::ScopedCritical critical; +#endif + // This prevents a race condition between thread A and B: + // 1. Thread A copies RefPtr, e.g. through assignment or copy constructor, + // gets the pointer from source, but is pre-empted before adding + // another reference + // 2. Thread B calls clear, leak_ref, or release_nonnull on source, and + // then drops the last reference, causing the object to be deleted + // 3. Thread A finishes step #1 by attempting to add a reference to + // the object that was already deleted in step #2 + FlatPtr bits = PtrTraits::lock(m_bits); + if (T* ptr = PtrTraits::as_ptr(bits)) + ptr->ref(); + PtrTraits::unlock(m_bits, bits); + return bits; + } + + ALWAYS_INLINE void assign_raw(FlatPtr bits) + { + FlatPtr prev_bits = PtrTraits::exchange(m_bits, bits); + unref_if_not_null(PtrTraits::as_ptr(prev_bits)); + } + + ALWAYS_INLINE T* as_ptr() const + { + return PtrTraits::as_ptr(m_bits.load(AK::MemoryOrder::memory_order_relaxed)); + } + + ALWAYS_INLINE T* as_nonnull_ptr() const + { + return as_nonnull_ptr(m_bits.load(AK::MemoryOrder::memory_order_relaxed)); + } + + ALWAYS_INLINE T* as_nonnull_ptr(FlatPtr bits) const + { + ASSERT(!PtrTraits::is_null(bits)); + return PtrTraits::as_ptr(bits); + } + + mutable Atomic m_bits { PtrTraits::default_null_value }; }; template> @@ -346,6 +487,12 @@ inline RefPtr static_ptr_cast(const RefPtr& ptr) return RefPtr(static_cast(ptr.ptr())); } +template +inline void swap(RefPtr& a, RefPtr& b) +{ + a.swap(b); +} + } using AK::RefPtr; diff --git a/AK/StdLibExtras.h b/AK/StdLibExtras.h index 0462ecd6991..25eadfb5412 100644 --- a/AK/StdLibExtras.h +++ b/AK/StdLibExtras.h @@ -146,6 +146,8 @@ struct IntegralConstant { typedef IntegralConstant FalseType; typedef IntegralConstant TrueType; +template +using VoidType = void; template struct IsLvalueReference : FalseType { @@ -302,7 +304,6 @@ template struct IsNullPointer : IsSame::Type> { }; - template struct RemoveReference { typedef T Type; diff --git a/AK/Tests/TestRefPtr.cpp b/AK/Tests/TestRefPtr.cpp index 93261ce2e00..52207f32685 100644 --- a/AK/Tests/TestRefPtr.cpp +++ b/AK/Tests/TestRefPtr.cpp @@ -33,6 +33,9 @@ struct Object : public RefCounted { int x; }; +struct Object2 : Object { +}; + TEST_CASE(basics) { RefPtr object = adopt(*new Object); @@ -67,6 +70,42 @@ TEST_CASE(assign_ptr) EXPECT_EQ(object->ref_count(), 1u); } +TEST_CASE(copy_move_ref) +{ + RefPtr object = adopt(*new Object2); + EXPECT_EQ(object->ref_count(), 1u); + { + auto object2 = object; + EXPECT_EQ(object->ref_count(), 2u); + + RefPtr object1 = object; + EXPECT_EQ(object->ref_count(), 3u); + + object1 = move(object2); + EXPECT_EQ(object->ref_count(), 2u); + + RefPtr object3(move(object1)); + EXPECT_EQ(object3->ref_count(), 2u); + + object1 = object3; + EXPECT_EQ(object3->ref_count(), 3u); + } + EXPECT_EQ(object->ref_count(), 1u); +} + +TEST_CASE(swap) +{ + RefPtr object_a = adopt(*new Object); + RefPtr object_b = adopt(*new Object); + auto* ptr_a = object_a.ptr(); + auto* ptr_b = object_b.ptr(); + swap(object_a, object_b); + EXPECT_EQ(object_a, ptr_b); + EXPECT_EQ(object_b, ptr_a); + EXPECT_EQ(object_a->ref_count(), 1u); + EXPECT_EQ(object_b->ref_count(), 1u); +} + TEST_CASE(assign_moved_self) { RefPtr object = adopt(*new Object); diff --git a/AK/Tests/TestWeakPtr.cpp b/AK/Tests/TestWeakPtr.cpp index 5ebe91570d4..a96942da080 100644 --- a/AK/Tests/TestWeakPtr.cpp +++ b/AK/Tests/TestWeakPtr.cpp @@ -35,7 +35,8 @@ # pragma clang diagnostic ignored "-Wunused-private-field" #endif -class SimpleWeakable : public Weakable { +class SimpleWeakable : public Weakable + , public RefCounted { public: SimpleWeakable() { } @@ -53,18 +54,18 @@ TEST_CASE(basic_weak) WeakPtr weak2; { - SimpleWeakable simple; - weak1 = simple.make_weak_ptr(); - weak2 = simple.make_weak_ptr(); + auto simple = adopt(*new SimpleWeakable); + weak1 = simple; + weak2 = simple; EXPECT_EQ(weak1.is_null(), false); EXPECT_EQ(weak2.is_null(), false); - EXPECT_EQ(weak1.ptr(), &simple); - EXPECT_EQ(weak1.ptr(), weak2.ptr()); + EXPECT_EQ(weak1.strong_ref().ptr(), simple.ptr()); + EXPECT_EQ(weak1.strong_ref().ptr(), weak2.strong_ref().ptr()); } EXPECT_EQ(weak1.is_null(), true); - EXPECT_EQ(weak1.ptr(), nullptr); - EXPECT_EQ(weak1.ptr(), weak2.ptr()); + EXPECT_EQ(weak1.strong_ref().ptr(), nullptr); + EXPECT_EQ(weak1.strong_ref().ptr(), weak2.strong_ref().ptr()); } TEST_CASE(weakptr_move) @@ -73,12 +74,12 @@ TEST_CASE(weakptr_move) WeakPtr weak2; { - SimpleWeakable simple; - weak1 = simple.make_weak_ptr(); + auto simple = adopt(*new SimpleWeakable); + weak1 = simple; weak2 = move(weak1); EXPECT_EQ(weak1.is_null(), true); EXPECT_EQ(weak2.is_null(), false); - EXPECT_EQ(weak2.ptr(), &simple); + EXPECT_EQ(weak2.strong_ref().ptr(), simple.ptr()); } EXPECT_EQ(weak2.is_null(), true); diff --git a/AK/WeakPtr.h b/AK/WeakPtr.h index 393d76576fe..bb460e32151 100644 --- a/AK/WeakPtr.h +++ b/AK/WeakPtr.h @@ -31,82 +31,209 @@ namespace AK { -template -class OwnPtr; - template class WeakPtr { - friend class Weakable; + template + friend class Weakable; public: WeakPtr() { } WeakPtr(std::nullptr_t) { } - template - WeakPtr(WeakPtr&& other) - : m_link(reinterpret_cast*>(other.take_link().ptr())) + template::value>::Type* = nullptr> + WeakPtr(const WeakPtr& other) + : m_link(other.m_link) { } - template + template::value>::Type* = nullptr> + WeakPtr(WeakPtr&& other) + : m_link(other.take_link()) + { + } + + template::value>::Type* = nullptr> WeakPtr& operator=(WeakPtr&& other) { - m_link = reinterpret_cast*>(other.take_link().ptr()); + m_link = other.take_link(); return *this; } - operator bool() const { return ptr(); } + template::value>::Type* = nullptr> + WeakPtr& operator=(const WeakPtr& other) + { + if ((const void*)this != (const void*)&other) + m_link = other.m_link; + return *this; + } - T* ptr() { return m_link ? m_link->ptr() : nullptr; } - const T* ptr() const { return m_link ? m_link->ptr() : nullptr; } + WeakPtr& operator=(std::nullptr_t) + { + clear(); + return *this; + } - T* operator->() { return ptr(); } - const T* operator->() const { return ptr(); } - - T& operator*() { return *ptr(); } - const T& operator*() const { return *ptr(); } - - operator const T*() const { return ptr(); } - operator T*() { return ptr(); } - - bool is_null() const { return !m_link || !m_link->ptr(); } - void clear() { m_link = nullptr; } - - RefPtr> take_link() { return move(m_link); } - - bool operator==(const OwnPtr& other) const { return ptr() == other.ptr(); } - -private: - WeakPtr(RefPtr> link) - : m_link(move(link)) + template::value>::Type* = nullptr> + WeakPtr(const U& object) + : m_link(object.template make_weak_ptr().take_link()) { } - RefPtr> m_link; + template::value>::Type* = nullptr> + WeakPtr(const U* object) + { + if (object) + m_link = object->template make_weak_ptr().take_link(); + } + + template::value>::Type* = nullptr> + WeakPtr(const RefPtr& object) + { + object.do_while_locked([&](U* obj) { + if (obj) + obj->template make_weak_ptr().take_link(); + }); + } + + template::value>::Type* = nullptr> + WeakPtr(const NonnullRefPtr& object) + { + object.do_while_locked([&](U* obj) { + if (obj) + obj->template make_weak_ptr().take_link(); + }); + } + + template::value>::Type* = nullptr> + WeakPtr& operator=(const U& object) + { + m_link = object.template make_weak_ptr().take_link(); + return *this; + } + + template::value>::Type* = nullptr> + WeakPtr& operator=(const U* object) + { + if (object) + m_link = object->template make_weak_ptr().take_link(); + else + m_link = nullptr; + return *this; + } + + template::value>::Type* = nullptr> + WeakPtr& operator=(const RefPtr& object) + { + object.do_while_locked([&](U* obj) { + if (obj) + m_link = obj->template make_weak_ptr().take_link(); + else + m_link = nullptr; + }); + return *this; + } + + template::value>::Type* = nullptr> + WeakPtr& operator=(const NonnullRefPtr& object) + { + object.do_while_locked([&](U* obj) { + if (obj) + m_link = obj->template make_weak_ptr().take_link(); + else + m_link = nullptr; + }); + return *this; + } + + RefPtr strong_ref() const + { + // This only works with RefCounted objects, but it is the only + // safe way to get a strong reference from a WeakPtr. Any code + // that uses objects not derived from RefCounted will have to + // use unsafe_ptr(), but as the name suggests, it is not safe... + RefPtr ref; + // Using do_while_locked protects against a race with clear()! + m_link.do_while_locked([&](WeakLink* link) { + if (link) + ref = link->template strong_ref(); + }); + return ref; + } + +#ifndef KERNEL + // A lot of user mode code is single-threaded. But for kernel mode code + // this is generally not true as everything is multi-threaded. So make + // these shortcuts and aliases only available to non-kernel code. + T* ptr() const { return unsafe_ptr(); } + T* operator->() { return unsafe_ptr(); } + const T* operator->() const { return unsafe_ptr(); } + operator const T*() const { return unsafe_ptr(); } + operator T*() { return unsafe_ptr(); } +#endif + + T* unsafe_ptr() const + { + T* ptr = nullptr; + m_link.do_while_locked([&](WeakLink* link) { + if (link) + ptr = link->unsafe_ptr(); + }); + return ptr; + } + + operator bool() const { return m_link ? !m_link->is_null() : false; } + + bool is_null() const { return !m_link || m_link->is_null(); } + void clear() { m_link = nullptr; } + + RefPtr take_link() { return move(m_link); } + +private: + WeakPtr(const RefPtr& link) + : m_link(link) + { + } + + RefPtr m_link; }; template -inline WeakPtr Weakable::make_weak_ptr() +template +inline WeakPtr Weakable::make_weak_ptr() const { #ifdef DEBUG ASSERT(!m_being_destroyed); #endif - if (!m_link) - m_link = adopt(*new WeakLink(static_cast(*this))); - return WeakPtr(m_link); + if (!m_link) { + // There is a small chance that we create a new WeakLink and throw + // it away because another thread beat us to it. But the window is + // pretty small and the overhead isn't terrible. + m_link.assign_if_null(adopt(*new WeakLink(const_cast(static_cast(*this))))); + } + return WeakPtr(m_link); } template inline const LogStream& operator<<(const LogStream& stream, const WeakPtr& value) { +#ifdef KERNEL + auto ref = value.strong_ref(); + return stream << ref.ptr(); +#else return stream << value.ptr(); +#endif } template struct Formatter> : Formatter { void format(TypeErasedFormatParams& params, FormatBuilder& builder, const WeakPtr& value) { +#ifdef KERNEL + auto ref = value.strong_ref(); + Formatter::format(params, builder, ref.ptr()); +#else Formatter::format(params, builder, value.ptr()); +#endif } }; diff --git a/AK/Weakable.h b/AK/Weakable.h index 5d40a216324..71c0ee9415d 100644 --- a/AK/Weakable.h +++ b/AK/Weakable.h @@ -27,6 +27,7 @@ #pragma once #include "Assertions.h" +#include "Atomic.h" #include "RefCounted.h" #include "RefPtr.h" @@ -41,20 +42,56 @@ class Weakable; template class WeakPtr; -template -class WeakLink : public RefCounted> { - friend class Weakable; +class WeakLink : public RefCounted { + template + friend class Weakable; + template + friend class WeakPtr; public: - T* ptr() { return m_ptr; } - const T* ptr() const { return m_ptr; } + template> + RefPtr strong_ref() const + { + RefPtr ref; + + { +#ifdef KERNEL + // We don't want to be pre-empted while we have the lock bit set + Kernel::ScopedCritical critical; +#endif + FlatPtr bits = RefPtrTraits::lock(m_bits); + T* ptr = static_cast(RefPtrTraits::as_ptr(bits)); + if (ptr) + ref = *ptr; + RefPtrTraits::unlock(m_bits, bits); + } + + return ref; + } + + template + T* unsafe_ptr() const + { + return static_cast(RefPtrTraits::as_ptr(m_bits.load(AK::MemoryOrder::memory_order_acquire))); + } + + bool is_null() const + { + return RefPtrTraits::is_null(m_bits.load(AK::MemoryOrder::memory_order_relaxed)); + } + + void revoke() + { + RefPtrTraits::exchange(m_bits, RefPtrTraits::default_null_value); + } private: + template explicit WeakLink(T& weakable) - : m_ptr(&weakable) + : m_bits(RefPtrTraits::as_bits(&weakable)) { } - T* m_ptr; + mutable Atomic m_bits; }; template @@ -63,7 +100,8 @@ private: class Link; public: - WeakPtr make_weak_ptr(); + template + WeakPtr make_weak_ptr() const; protected: Weakable() { } @@ -79,11 +117,11 @@ protected: void revoke_weak_ptrs() { if (m_link) - m_link->m_ptr = nullptr; + m_link->revoke(); } private: - RefPtr> m_link; + mutable RefPtr m_link; #ifdef WEAKABLE_DEBUG bool m_being_destroyed { false }; #endif diff --git a/Applications/PixelPaint/Tool.cpp b/Applications/PixelPaint/Tool.cpp index 415bcb15360..846e0174102 100644 --- a/Applications/PixelPaint/Tool.cpp +++ b/Applications/PixelPaint/Tool.cpp @@ -40,7 +40,7 @@ Tool::~Tool() void Tool::setup(ImageEditor& editor) { - m_editor = editor.make_weak_ptr(); + m_editor = editor; } void Tool::set_action(GUI::Action* action) diff --git a/Applications/Spreadsheet/Spreadsheet.cpp b/Applications/Spreadsheet/Spreadsheet.cpp index 07ed58ba3d5..d4d6dea1bb0 100644 --- a/Applications/Spreadsheet/Spreadsheet.cpp +++ b/Applications/Spreadsheet/Spreadsheet.cpp @@ -345,12 +345,12 @@ RefPtr Sheet::from_json(const JsonObject& object, Workbook& workbook) OwnPtr cell; switch (kind) { case Cell::LiteralString: - cell = make(obj.get("value").to_string(), position, sheet->make_weak_ptr()); + cell = make(obj.get("value").to_string(), position, *sheet); break; case Cell::Formula: { auto& interpreter = sheet->interpreter(); auto value = interpreter.vm().call(parse_function, json, JS::js_string(interpreter.heap(), obj.get("value").as_string())); - cell = make(obj.get("source").to_string(), move(value), position, sheet->make_weak_ptr()); + cell = make(obj.get("source").to_string(), move(value), position, *sheet); break; } } diff --git a/Applications/Spreadsheet/Spreadsheet.h b/Applications/Spreadsheet/Spreadsheet.h index 4748e798793..a02ed777408 100644 --- a/Applications/Spreadsheet/Spreadsheet.h +++ b/Applications/Spreadsheet/Spreadsheet.h @@ -82,7 +82,7 @@ public: if (auto cell = at(position)) return *cell; - m_cells.set(position, make(String::empty(), position, make_weak_ptr())); + m_cells.set(position, make(String::empty(), position, *this)); return *at(position); } diff --git a/DevTools/HackStudio/Editor.cpp b/DevTools/HackStudio/Editor.cpp index 9b7951b7c1c..4a081068e47 100644 --- a/DevTools/HackStudio/Editor.cpp +++ b/DevTools/HackStudio/Editor.cpp @@ -61,7 +61,7 @@ Editor::Editor() m_documentation_tooltip_window->set_window_type(GUI::WindowType::Tooltip); m_documentation_page_view = m_documentation_tooltip_window->set_main_widget(); - m_autocomplete_box = make(make_weak_ptr()); + m_autocomplete_box = make(*this); } Editor::~Editor() diff --git a/Kernel/FileSystem/DevPtsFS.cpp b/Kernel/FileSystem/DevPtsFS.cpp index d05437591a8..3c781e73cd3 100644 --- a/Kernel/FileSystem/DevPtsFS.cpp +++ b/Kernel/FileSystem/DevPtsFS.cpp @@ -113,7 +113,7 @@ DevPtsFSInode::DevPtsFSInode(DevPtsFS& fs, unsigned index, SlavePTY* pty) : Inode(fs, index) { if (pty) - m_pty = pty->make_weak_ptr(); + m_pty = *pty; } DevPtsFSInode::~DevPtsFSInode() @@ -132,9 +132,9 @@ ssize_t DevPtsFSInode::write_bytes(off_t, ssize_t, const UserOrKernelBuffer&, Fi InodeMetadata DevPtsFSInode::metadata() const { - if (m_pty) { + if (auto pty = m_pty.strong_ref()) { auto metadata = m_metadata; - metadata.mtime = m_pty->time_of_last_write(); + metadata.mtime = pty->time_of_last_write(); return metadata; } return m_metadata; diff --git a/Kernel/FileSystem/Inode.cpp b/Kernel/FileSystem/Inode.cpp index 62e425dccb4..106c39ffd6c 100644 --- a/Kernel/FileSystem/Inode.cpp +++ b/Kernel/FileSystem/Inode.cpp @@ -129,14 +129,14 @@ void Inode::will_be_destroyed() void Inode::inode_contents_changed(off_t offset, ssize_t size, const UserOrKernelBuffer& data) { - if (m_shared_vmobject) - m_shared_vmobject->inode_contents_changed({}, offset, size, data); + if (auto shared_vmobject = this->shared_vmobject()) + shared_vmobject->inode_contents_changed({}, offset, size, data); } void Inode::inode_size_changed(size_t old_size, size_t new_size) { - if (m_shared_vmobject) - m_shared_vmobject->inode_size_changed({}, old_size, new_size); + if (auto shared_vmobject = this->shared_vmobject()) + shared_vmobject->inode_size_changed({}, old_size, new_size); } int Inode::set_atime(time_t) @@ -166,7 +166,7 @@ KResult Inode::decrement_link_count() void Inode::set_shared_vmobject(SharedInodeVMObject& vmobject) { - m_shared_vmobject = vmobject.make_weak_ptr(); + m_shared_vmobject = vmobject; } bool Inode::bind_socket(LocalSocket& socket) @@ -260,4 +260,19 @@ KResult Inode::prepare_to_write_data() return KSuccess; } +RefPtr Inode::shared_vmobject() +{ + return m_shared_vmobject.strong_ref(); +} + +RefPtr Inode::shared_vmobject() const +{ + return m_shared_vmobject.strong_ref(); +} + +bool Inode::is_shared_vmobject(const SharedInodeVMObject& other) const +{ + return m_shared_vmobject.unsafe_ptr() == &other; +} + } diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h index 62962bedf58..2567e6fcc4d 100644 --- a/Kernel/FileSystem/Inode.h +++ b/Kernel/FileSystem/Inode.h @@ -102,8 +102,9 @@ public: void will_be_destroyed(); void set_shared_vmobject(SharedInodeVMObject&); - SharedInodeVMObject* shared_vmobject() { return m_shared_vmobject.ptr(); } - const SharedInodeVMObject* shared_vmobject() const { return m_shared_vmobject.ptr(); } + RefPtr shared_vmobject(); + RefPtr shared_vmobject() const; + bool is_shared_vmobject(const SharedInodeVMObject&) const; static InlineLinkedList& all_with_lock(); static void sync(); diff --git a/Kernel/FileSystem/InodeWatcher.cpp b/Kernel/FileSystem/InodeWatcher.cpp index 193095de4d8..043396986e2 100644 --- a/Kernel/FileSystem/InodeWatcher.cpp +++ b/Kernel/FileSystem/InodeWatcher.cpp @@ -36,15 +36,15 @@ NonnullRefPtr InodeWatcher::create(Inode& inode) } InodeWatcher::InodeWatcher(Inode& inode) - : m_inode(inode.make_weak_ptr()) + : m_inode(inode) { inode.register_watcher({}, *this); } InodeWatcher::~InodeWatcher() { - if (RefPtr safe_inode = m_inode.ptr()) - safe_inode->unregister_watcher({}, *this); + if (auto inode = m_inode.strong_ref()) + inode->unregister_watcher({}, *this); } bool InodeWatcher::can_read(const FileDescription&, size_t) const @@ -88,9 +88,9 @@ KResultOr InodeWatcher::write(FileDescription&, size_t, const UserOrKern String InodeWatcher::absolute_path(const FileDescription&) const { - if (!m_inode) - return "InodeWatcher:(gone)"; - return String::format("InodeWatcher:%s", m_inode->identifier().to_string().characters()); + if (auto inode = m_inode.strong_ref()) + return String::format("InodeWatcher:%s", inode->identifier().to_string().characters()); + return "InodeWatcher:(gone)"; } void InodeWatcher::notify_inode_event(Badge, Event::Type event_type) diff --git a/Kernel/Net/TCPSocket.cpp b/Kernel/Net/TCPSocket.cpp index 44e7f062f53..8f66216dd40 100644 --- a/Kernel/Net/TCPSocket.cpp +++ b/Kernel/Net/TCPSocket.cpp @@ -130,7 +130,7 @@ RefPtr TCPSocket::create_client(const IPv4Address& new_local_address, void TCPSocket::release_to_originator() { ASSERT(!!m_originator); - m_originator->release_for_accept(this); + m_originator.strong_ref()->release_for_accept(this); } void TCPSocket::release_for_accept(RefPtr socket) diff --git a/Kernel/Net/TCPSocket.h b/Kernel/Net/TCPSocket.h index 0a8e25119d2..452de7deeca 100644 --- a/Kernel/Net/TCPSocket.h +++ b/Kernel/Net/TCPSocket.h @@ -159,7 +159,7 @@ public: static Lockable>>& closing_sockets(); RefPtr create_client(const IPv4Address& local_address, u16 local_port, const IPv4Address& peer_address, u16 peer_port); - void set_originator(TCPSocket& originator) { m_originator = originator.make_weak_ptr(); } + void set_originator(TCPSocket& originator) { m_originator = originator; } bool has_originator() { return !!m_originator; } void release_to_originator(); void release_for_accept(RefPtr); diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 717b608779f..21745ac09b8 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -194,7 +194,7 @@ bool Process::deallocate_region(Region& region) OwnPtr region_protector; ScopedSpinLock lock(m_lock); - if (m_region_lookup_cache.region == ®ion) + if (m_region_lookup_cache.region.unsafe_ptr() == ®ion) m_region_lookup_cache.region = nullptr; for (size_t i = 0; i < m_regions.size(); ++i) { if (&m_regions[i] == ®ion) { @@ -209,13 +209,13 @@ Region* Process::find_region_from_range(const Range& range) { ScopedSpinLock lock(m_lock); if (m_region_lookup_cache.range == range && m_region_lookup_cache.region) - return m_region_lookup_cache.region; + return m_region_lookup_cache.region.unsafe_ptr(); size_t size = PAGE_ROUND_UP(range.size()); for (auto& region : m_regions) { if (region.vaddr() == range.base() && region.size() == size) { m_region_lookup_cache.range = range; - m_region_lookup_cache.region = region.make_weak_ptr(); + m_region_lookup_cache.region = region; return ®ion; } } diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index 2b91bad29c8..b2bb8a17c51 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -485,6 +485,7 @@ bool Scheduler::pick_next() Thread* thread_to_schedule = nullptr; + auto pending_beneficiary = scheduler_data.m_pending_beneficiary.strong_ref(); Vector sorted_runnables; for_each_runnable([&](auto& thread) { if ((thread.affinity() & (1u << Processor::current().id())) == 0) @@ -492,7 +493,7 @@ bool Scheduler::pick_next() if (thread.state() == Thread::Running && &thread != current_thread) return IterationDecision::Continue; sorted_runnables.append(&thread); - if (&thread == scheduler_data.m_pending_beneficiary) { + if (&thread == pending_beneficiary) { thread_to_schedule = &thread; return IterationDecision::Break; } @@ -628,7 +629,7 @@ bool Scheduler::donate_to(RefPtr& beneficiary, const char* reason) ASSERT(!proc.in_irq()); if (proc.in_critical() > 1) { - scheduler_data.m_pending_beneficiary = beneficiary->make_weak_ptr(); // Save the beneficiary + scheduler_data.m_pending_beneficiary = *beneficiary; // Save the beneficiary scheduler_data.m_pending_donate_reason = reason; proc.invoke_scheduler_async(); return false; diff --git a/Kernel/SharedBuffer.cpp b/Kernel/SharedBuffer.cpp index 141204df3e4..29223bcda7a 100644 --- a/Kernel/SharedBuffer.cpp +++ b/Kernel/SharedBuffer.cpp @@ -92,13 +92,13 @@ void* SharedBuffer::ref_for_process_and_get_address(Process& process) auto* region = process.allocate_region_with_vmobject(VirtualAddress(), size(), m_vmobject, 0, "SharedBuffer", PROT_READ | (m_writable ? PROT_WRITE : 0)); if (!region) return (void*)-ENOMEM; - ref.region = region->make_weak_ptr(); - ref.region->set_shared(true); + ref.region = region; + region->set_shared(true); } ref.count++; m_total_refs++; sanity_check("ref_for_process_and_get_address"); - return ref.region->vaddr().as_ptr(); + return ref.region.unsafe_ptr()->vaddr().as_ptr(); // TODO: Region needs to be RefCounted! } } ASSERT_NOT_REACHED(); @@ -133,7 +133,7 @@ void SharedBuffer::deref_for_process(Process& process) #ifdef SHARED_BUFFER_DEBUG dbg() << "Releasing shared buffer reference on " << m_shbuf_id << " of size " << size() << " by PID " << process.pid().value(); #endif - process.deallocate_region(*ref.region); + process.deallocate_region(*ref.region.unsafe_ptr()); // TODO: Region needs to be RefCounted! #ifdef SHARED_BUFFER_DEBUG dbg() << "Released shared buffer reference on " << m_shbuf_id << " of size " << size() << " by PID " << process.pid().value(); #endif @@ -187,9 +187,10 @@ void SharedBuffer::seal() LOCKER(shared_buffers().lock()); m_writable = false; for (auto& ref : m_refs) { - if (ref.region) { - ref.region->set_writable(false); - ref.region->remap(); + // TODO: Region needs to be RefCounted! + if (auto* region = ref.region.unsafe_ptr()) { + region->set_writable(false); + region->remap(); } } } diff --git a/Kernel/Syscalls/fork.cpp b/Kernel/Syscalls/fork.cpp index 161817a373d..86ccdddab53 100644 --- a/Kernel/Syscalls/fork.cpp +++ b/Kernel/Syscalls/fork.cpp @@ -88,8 +88,8 @@ pid_t Process::sys$fork(RegisterState& regs) auto& child_region = child->add_region(region.clone()); child_region.map(child->page_directory()); - if (®ion == m_master_tls_region) - child->m_master_tls_region = child_region.make_weak_ptr(); + if (®ion == m_master_tls_region.unsafe_ptr()) + child->m_master_tls_region = child_region; } ScopedSpinLock processes_lock(g_processes_lock); diff --git a/Kernel/TTY/TTY.cpp b/Kernel/TTY/TTY.cpp index 7e9d8aefcf0..69617991c9f 100644 --- a/Kernel/TTY/TTY.cpp +++ b/Kernel/TTY/TTY.cpp @@ -166,8 +166,8 @@ void TTY::emit(u8 ch) if (ch == m_termios.c_cc[VSUSP]) { dbg() << tty_name() << ": VSUSP pressed!"; generate_signal(SIGTSTP); - if (m_original_process_parent) - (void)m_original_process_parent->send_signal(SIGCHLD, nullptr); + if (auto original_process_parent = m_original_process_parent.strong_ref()) + (void)original_process_parent->send_signal(SIGCHLD, nullptr); // TODO: Else send it to the session leader maybe? return; } @@ -330,11 +330,11 @@ int TTY::ioctl(FileDescription&, unsigned request, FlatPtr arg) return -EPERM; if (process && pgid != process->pgid()) return -EPERM; - m_pg = process_group->make_weak_ptr(); + m_pg = *process_group; if (process) { if (auto parent = Process::from_pid(process->ppid())) { - m_original_process_parent = parent->make_weak_ptr(); + m_original_process_parent = *parent; return 0; } } diff --git a/Kernel/TTY/TTY.h b/Kernel/TTY/TTY.h index 6b9d5713360..aafbe17292d 100644 --- a/Kernel/TTY/TTY.h +++ b/Kernel/TTY/TTY.h @@ -51,7 +51,12 @@ public: unsigned short rows() const { return m_rows; } unsigned short columns() const { return m_columns; } - ProcessGroupID pgid() const { return m_pg ? m_pg->pgid() : 0; } + ProcessGroupID pgid() const + { + if (auto pg = m_pg.strong_ref()) + return pg->pgid(); + return 0; + } void set_termios(const termios&); bool should_generate_signals() const { return m_termios.c_lflag & ISIG; } diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index 9fd13bde704..1dd2c2862c7 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -978,7 +978,7 @@ KResult Thread::make_thread_specific_region(Badge) m_thread_specific_data = VirtualAddress(thread_specific_data); thread_specific_data->self = thread_specific_data; if (process().m_master_tls_size) - memcpy(thread_local_storage, process().m_master_tls_region->vaddr().as_ptr(), process().m_master_tls_size); + memcpy(thread_local_storage, process().m_master_tls_region.unsafe_ptr()->vaddr().as_ptr(), process().m_master_tls_size); return KSuccess; } diff --git a/Kernel/VM/SharedInodeVMObject.cpp b/Kernel/VM/SharedInodeVMObject.cpp index c1fb4c09263..d3452ee4a87 100644 --- a/Kernel/VM/SharedInodeVMObject.cpp +++ b/Kernel/VM/SharedInodeVMObject.cpp @@ -58,7 +58,7 @@ SharedInodeVMObject::SharedInodeVMObject(const SharedInodeVMObject& other) SharedInodeVMObject::~SharedInodeVMObject() { - ASSERT(inode().shared_vmobject() == this); + ASSERT(inode().is_shared_vmobject(*this)); } } diff --git a/Libraries/LibCore/Event.cpp b/Libraries/LibCore/Event.cpp index 67bdd3dff8e..e7de1886b4c 100644 --- a/Libraries/LibCore/Event.cpp +++ b/Libraries/LibCore/Event.cpp @@ -40,4 +40,32 @@ ChildEvent::~ChildEvent() { } +Object* ChildEvent::child() +{ + if (auto ref = m_child.strong_ref()) + return ref.ptr(); + return nullptr; +} + +const Object* ChildEvent::child() const +{ + if (auto ref = m_child.strong_ref()) + return ref.ptr(); + return nullptr; +} + +Object* ChildEvent::insertion_before_child() +{ + if (auto ref = m_insertion_before_child.strong_ref()) + return ref.ptr(); + return nullptr; +} + +const Object* ChildEvent::insertion_before_child() const +{ + if (auto ref = m_insertion_before_child.strong_ref()) + return ref.ptr(); + return nullptr; +} + } diff --git a/Libraries/LibCore/Event.h b/Libraries/LibCore/Event.h index dfa0f4fe3e6..46dc3dc3750 100644 --- a/Libraries/LibCore/Event.h +++ b/Libraries/LibCore/Event.h @@ -130,11 +130,11 @@ public: ChildEvent(Type, Object& child, Object* insertion_before_child = nullptr); ~ChildEvent(); - Object* child() { return m_child.ptr(); } - const Object* child() const { return m_child.ptr(); } + Object* child(); + const Object* child() const; - Object* insertion_before_child() { return m_insertion_before_child.ptr(); } - const Object* insertion_before_child() const { return m_insertion_before_child.ptr(); } + Object* insertion_before_child(); + const Object* insertion_before_child() const; private: WeakPtr m_child; diff --git a/Libraries/LibCore/EventLoop.cpp b/Libraries/LibCore/EventLoop.cpp index 97a9ce26bc2..ebd61b3a37e 100644 --- a/Libraries/LibCore/EventLoop.cpp +++ b/Libraries/LibCore/EventLoop.cpp @@ -122,8 +122,8 @@ public: } virtual ~RPCClient() override { - if (m_inspected_object) - m_inspected_object->decrement_inspector_count({}); + if (auto inspected_object = m_inspected_object.strong_ref()) + inspected_object->decrement_inspector_count({}); } void send_response(const JsonObject& response) @@ -177,10 +177,10 @@ public: auto address = request.get("address").to_number(); for (auto& object : Object::all_objects()) { if ((FlatPtr)&object == address) { - if (m_inspected_object) - m_inspected_object->decrement_inspector_count({}); - m_inspected_object = object.make_weak_ptr(); - m_inspected_object->increment_inspector_count({}); + if (auto inspected_object = m_inspected_object.strong_ref()) + inspected_object->decrement_inspector_count({}); + m_inspected_object = object; + object.increment_inspector_count({}); break; } } @@ -364,7 +364,7 @@ void EventLoop::pump(WaitMode mode) for (size_t i = 0; i < events.size(); ++i) { auto& queued_event = events.at(i); - auto* receiver = queued_event.receiver.ptr(); + auto receiver = queued_event.receiver.strong_ref(); auto& event = *queued_event.event; #ifdef EVENTLOOP_DEBUG if (receiver) @@ -639,15 +639,16 @@ try_select_again: auto& timer = *it.value; if (!timer.has_expired(now)) continue; - if (it.value->fire_when_not_visible == TimerShouldFireWhenNotVisible::No - && it.value->owner - && !it.value->owner->is_visible_for_timer_purposes()) { + auto owner = timer.owner.strong_ref(); + if (timer.fire_when_not_visible == TimerShouldFireWhenNotVisible::No + && owner && !owner->is_visible_for_timer_purposes()) { continue; } #ifdef EVENTLOOP_DEBUG - dbgln("Core::EventLoop: Timer {} has expired, sending Core::TimerEvent to {}", timer.timer_id, timer.owner); + dbgln("Core::EventLoop: Timer {} has expired, sending Core::TimerEvent to {}", timer.timer_id, *owner); #endif - post_event(*timer.owner, make(timer.timer_id)); + if (owner) + post_event(*owner, make(timer.timer_id)); if (timer.should_reload) { timer.reload(now); } else { @@ -688,9 +689,9 @@ Optional EventLoop::get_next_timer_expiration() Optional soonest {}; for (auto& it : *s_timers) { auto& fire_time = it.value->fire_time; + auto owner = it.value->owner.strong_ref(); if (it.value->fire_when_not_visible == TimerShouldFireWhenNotVisible::No - && it.value->owner - && !it.value->owner->is_visible_for_timer_purposes()) { + && owner && !owner->is_visible_for_timer_purposes()) { continue; } if (!soonest.has_value() || fire_time.tv_sec < soonest.value().tv_sec || (fire_time.tv_sec == soonest.value().tv_sec && fire_time.tv_usec < soonest.value().tv_usec)) @@ -703,7 +704,7 @@ int EventLoop::register_timer(Object& object, int milliseconds, bool should_relo { ASSERT(milliseconds >= 0); auto timer = make(); - timer->owner = object.make_weak_ptr(); + timer->owner = object; timer->interval = milliseconds; timeval now; timespec now_spec; @@ -750,7 +751,7 @@ void EventLoop::wake() } EventLoop::QueuedEvent::QueuedEvent(Object& receiver, NonnullOwnPtr event) - : receiver(receiver.make_weak_ptr()) + : receiver(receiver) , event(move(event)) { } diff --git a/Libraries/LibGUI/Application.cpp b/Libraries/LibGUI/Application.cpp index 94fb0b3c44f..18837aeb077 100644 --- a/Libraries/LibGUI/Application.cpp +++ b/Libraries/LibGUI/Application.cpp @@ -50,7 +50,7 @@ Application* Application::the() Application::Application(int argc, char** argv) { ASSERT(!*s_the); - *s_the = make_weak_ptr(); + *s_the = *this; m_event_loop = make(); WindowServerConnection::the(); Clipboard::initialize({}); diff --git a/Libraries/LibGUI/Button.cpp b/Libraries/LibGUI/Button.cpp index 7809e908d31..f488c272b86 100644 --- a/Libraries/LibGUI/Button.cpp +++ b/Libraries/LibGUI/Button.cpp @@ -129,7 +129,7 @@ void Button::context_menu_event(ContextMenuEvent& context_menu_event) void Button::set_action(Action& action) { - m_action = action.make_weak_ptr(); + m_action = action; action.register_button({}, *this); set_enabled(action.is_enabled()); set_checkable(action.is_checkable()); diff --git a/Libraries/LibGUI/Layout.cpp b/Libraries/LibGUI/Layout.cpp index 8616a99252a..250b1c5ca45 100644 --- a/Libraries/LibGUI/Layout.cpp +++ b/Libraries/LibGUI/Layout.cpp @@ -82,7 +82,7 @@ void Layout::notify_adopted(Badge, Widget& widget) { if (m_owner == &widget) return; - m_owner = widget.make_weak_ptr(); + m_owner = widget; } void Layout::notify_disowned(Badge, Widget& widget) @@ -117,7 +117,7 @@ void Layout::add_widget(Widget& widget) { Entry entry; entry.type = Entry::Type::Widget; - entry.widget = widget.make_weak_ptr(); + entry.widget = widget; add_entry(move(entry)); } @@ -125,7 +125,7 @@ void Layout::insert_widget_before(Widget& widget, Widget& before_widget) { Entry entry; entry.type = Entry::Type::Widget; - entry.widget = widget.make_weak_ptr(); + entry.widget = widget; m_entries.insert_before_matching(move(entry), [&](auto& existing_entry) { return existing_entry.type == Entry::Type::Widget && existing_entry.widget.ptr() == &before_widget; }); diff --git a/Libraries/LibGUI/Menu.cpp b/Libraries/LibGUI/Menu.cpp index 445a95394f9..a32dd38078f 100644 --- a/Libraries/LibGUI/Menu.cpp +++ b/Libraries/LibGUI/Menu.cpp @@ -162,7 +162,7 @@ int Menu::realize_menu(RefPtr default_action) } } all_menus().set(m_menu_id, this); - m_last_default_action = default_action ? default_action->make_weak_ptr() : nullptr; + m_last_default_action = default_action; return m_menu_id; } diff --git a/Libraries/LibGUI/Splitter.cpp b/Libraries/LibGUI/Splitter.cpp index d99e735b8d3..faeec39aa62 100644 --- a/Libraries/LibGUI/Splitter.cpp +++ b/Libraries/LibGUI/Splitter.cpp @@ -124,8 +124,8 @@ void Splitter::mousedown_event(MouseEvent& event) if (!get_resize_candidates_at(event.position(), first, second)) return; - m_first_resizee = first->make_weak_ptr(); - m_second_resizee = second->make_weak_ptr(); + m_first_resizee = *first; + m_second_resizee = *second; m_first_resizee_start_size = first->size(); m_second_resizee_start_size = second->size(); m_resize_origin = event.position(); diff --git a/Libraries/LibGUI/SyntaxHighlighter.cpp b/Libraries/LibGUI/SyntaxHighlighter.cpp index 7db2da49855..5286758419e 100644 --- a/Libraries/LibGUI/SyntaxHighlighter.cpp +++ b/Libraries/LibGUI/SyntaxHighlighter.cpp @@ -128,7 +128,7 @@ void SyntaxHighlighter::highlight_matching_token_pair() void SyntaxHighlighter::attach(TextEditor& editor) { ASSERT(!m_editor); - m_editor = editor.make_weak_ptr(); + m_editor = editor; } void SyntaxHighlighter::detach() diff --git a/Libraries/LibGUI/Widget.cpp b/Libraries/LibGUI/Widget.cpp index 69327e5c508..10a21e0fe89 100644 --- a/Libraries/LibGUI/Widget.cpp +++ b/Libraries/LibGUI/Widget.cpp @@ -533,10 +533,7 @@ void Widget::set_focus_proxy(Widget* proxy) if (m_focus_proxy == proxy) return; - if (proxy) - m_focus_proxy = proxy->make_weak_ptr(); - else - m_focus_proxy = nullptr; + m_focus_proxy = proxy; } FocusPolicy Widget::focus_policy() const diff --git a/Libraries/LibGUI/Window.cpp b/Libraries/LibGUI/Window.cpp index 50e44912bbb..6faaa5a08a8 100644 --- a/Libraries/LibGUI/Window.cpp +++ b/Libraries/LibGUI/Window.cpp @@ -297,7 +297,7 @@ void Window::handle_mouse_event(MouseEvent& event) ASSERT(result.widget); set_hovered_widget(result.widget); if (event.buttons() != 0 && !m_automatic_cursor_tracking_widget) - m_automatic_cursor_tracking_widget = result.widget->make_weak_ptr(); + m_automatic_cursor_tracking_widget = *result.widget; if (result.widget != m_global_cursor_tracking_widget.ptr()) return result.widget->dispatch_event(*local_event, this); return; @@ -562,7 +562,7 @@ void Window::set_focused_widget(Widget* widget, FocusSource source) Core::EventLoop::current().post_event(*m_focused_widget, make(Event::FocusOut, source)); m_focused_widget->update(); } - m_focused_widget = widget ? widget->make_weak_ptr() : nullptr; + m_focused_widget = widget; if (m_focused_widget) { Core::EventLoop::current().post_event(*m_focused_widget, make(Event::FocusIn, source)); m_focused_widget->update(); @@ -573,14 +573,14 @@ void Window::set_global_cursor_tracking_widget(Widget* widget) { if (widget == m_global_cursor_tracking_widget) return; - m_global_cursor_tracking_widget = widget ? widget->make_weak_ptr() : nullptr; + m_global_cursor_tracking_widget = widget; } void Window::set_automatic_cursor_tracking_widget(Widget* widget) { if (widget == m_automatic_cursor_tracking_widget) return; - m_automatic_cursor_tracking_widget = widget ? widget->make_weak_ptr() : nullptr; + m_automatic_cursor_tracking_widget = widget; } void Window::set_has_alpha_channel(bool value) @@ -621,7 +621,7 @@ void Window::set_hovered_widget(Widget* widget) if (m_hovered_widget) Core::EventLoop::current().post_event(*m_hovered_widget, make(Event::Leave)); - m_hovered_widget = widget ? widget->make_weak_ptr() : nullptr; + m_hovered_widget = widget; if (m_hovered_widget) Core::EventLoop::current().post_event(*m_hovered_widget, make(Event::Enter)); diff --git a/Libraries/LibProtocol/Download.cpp b/Libraries/LibProtocol/Download.cpp index 3ed284ff9b9..05260a9636f 100644 --- a/Libraries/LibProtocol/Download.cpp +++ b/Libraries/LibProtocol/Download.cpp @@ -31,7 +31,7 @@ namespace Protocol { Download::Download(Client& client, i32 download_id) - : m_client(client.make_weak_ptr()) + : m_client(client) , m_download_id(download_id) { } diff --git a/Libraries/LibWeb/CSS/StyleValue.cpp b/Libraries/LibWeb/CSS/StyleValue.cpp index 164670ab39d..c744e290533 100644 --- a/Libraries/LibWeb/CSS/StyleValue.cpp +++ b/Libraries/LibWeb/CSS/StyleValue.cpp @@ -290,7 +290,7 @@ Color IdentifierStyleValue::to_color(const DOM::Document& document) const ImageStyleValue::ImageStyleValue(const URL& url, DOM::Document& document) : StyleValue(Type::Image) , m_url(url) - , m_document(document.make_weak_ptr()) + , m_document(document) { LoadRequest request; request.set_url(url); diff --git a/Libraries/LibWeb/DOM/Document.cpp b/Libraries/LibWeb/DOM/Document.cpp index 3fa820aa106..4f2084fe0ee 100644 --- a/Libraries/LibWeb/DOM/Document.cpp +++ b/Libraries/LibWeb/DOM/Document.cpp @@ -234,7 +234,7 @@ String Document::title() const void Document::attach_to_frame(Badge, Frame& frame) { - m_frame = frame.make_weak_ptr(); + m_frame = frame; for_each_in_subtree([&](auto& node) { node.document_did_attach_to_frame(frame); return IterationDecision::Continue; @@ -584,10 +584,7 @@ void Document::set_focused_element(Element* element) if (m_focused_element == element) return; - if (element) - m_focused_element = element->make_weak_ptr(); - else - m_focused_element = nullptr; + m_focused_element = element; if (m_layout_root) m_layout_root->set_needs_display(); diff --git a/Libraries/LibWeb/HTML/CanvasRenderingContext2D.cpp b/Libraries/LibWeb/HTML/CanvasRenderingContext2D.cpp index ae2579e3bca..729e78ec57b 100644 --- a/Libraries/LibWeb/HTML/CanvasRenderingContext2D.cpp +++ b/Libraries/LibWeb/HTML/CanvasRenderingContext2D.cpp @@ -35,7 +35,7 @@ namespace Web::HTML { CanvasRenderingContext2D::CanvasRenderingContext2D(HTMLCanvasElement& element) - : m_element(element.make_weak_ptr()) + : m_element(element) { } diff --git a/Libraries/LibWeb/HTML/HTMLScriptElement.cpp b/Libraries/LibWeb/HTML/HTMLScriptElement.cpp index 1b2bec1e734..524d9d742c4 100644 --- a/Libraries/LibWeb/HTML/HTMLScriptElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLScriptElement.cpp @@ -45,7 +45,7 @@ HTMLScriptElement::~HTMLScriptElement() void HTMLScriptElement::set_parser_document(Badge, DOM::Document& document) { - m_parser_document = document.make_weak_ptr(); + m_parser_document = document; } void HTMLScriptElement::set_non_blocking(Badge, bool non_blocking) @@ -82,12 +82,12 @@ void HTMLScriptElement::prepare_script(Badge) // FIXME: Check the "type" and "language" attributes if (parser_document) { - m_parser_document = parser_document->make_weak_ptr(); + m_parser_document = *parser_document; m_non_blocking = false; } m_already_started = true; - m_preparation_time_document = document().make_weak_ptr(); + m_preparation_time_document = document(); if (parser_document && parser_document.ptr() != m_preparation_time_document.ptr()) { return; diff --git a/Libraries/LibWeb/Page/Frame.cpp b/Libraries/LibWeb/Page/Frame.cpp index fafc0d0f931..ff501d0d73f 100644 --- a/Libraries/LibWeb/Page/Frame.cpp +++ b/Libraries/LibWeb/Page/Frame.cpp @@ -40,7 +40,7 @@ Frame::Frame(DOM::Element& host_element, Frame& main_frame) , m_main_frame(main_frame) , m_loader(*this) , m_event_handler({}, *this) - , m_host_element(host_element.make_weak_ptr()) + , m_host_element(host_element) { setup(); } diff --git a/Services/AudioServer/Mixer.cpp b/Services/AudioServer/Mixer.cpp index 782168d962b..6d72a1aa8d0 100644 --- a/Services/AudioServer/Mixer.cpp +++ b/Services/AudioServer/Mixer.cpp @@ -148,7 +148,7 @@ void Mixer::set_muted(bool muted) } BufferQueue::BufferQueue(ClientConnection& client) - : m_client(client.make_weak_ptr()) + : m_client(client) { } diff --git a/Services/WindowServer/AppletManager.cpp b/Services/WindowServer/AppletManager.cpp index 18b50cda30c..0e854143617 100644 --- a/Services/WindowServer/AppletManager.cpp +++ b/Services/WindowServer/AppletManager.cpp @@ -69,7 +69,7 @@ void AppletManager::event(Core::Event& event) void AppletManager::add_applet(Window& applet) { - m_applets.append(applet.make_weak_ptr()); + m_applets.append(applet); // Prune any dead weak pointers from the applet list. m_applets.remove_all_matching([](auto& entry) { diff --git a/Services/WindowServer/Menu.h b/Services/WindowServer/Menu.h index 0a77eedcf16..7b5f37ffd67 100644 --- a/Services/WindowServer/Menu.h +++ b/Services/WindowServer/Menu.h @@ -83,7 +83,7 @@ public: Window& ensure_menu_window(); Window* window_menu_of() { return m_window_menu_of; } - void set_window_menu_of(Window& window) { m_window_menu_of = window.make_weak_ptr(); } + void set_window_menu_of(Window& window) { m_window_menu_of = window; } bool is_window_menu_open() { return m_is_window_menu_open; } void set_window_menu_open(bool is_open) { m_is_window_menu_open = is_open; } diff --git a/Services/WindowServer/MenuManager.cpp b/Services/WindowServer/MenuManager.cpp index 7ae2c73dac1..abe9d0cfc48 100644 --- a/Services/WindowServer/MenuManager.cpp +++ b/Services/WindowServer/MenuManager.cpp @@ -392,7 +392,7 @@ void MenuManager::open_menu(Menu& menu, bool as_current_menu) } if (m_open_menu_stack.find([&menu](auto& other) { return &menu == other.ptr(); }).is_end()) - m_open_menu_stack.append(menu.make_weak_ptr()); + m_open_menu_stack.append(menu); if (as_current_menu || !current_menu()) { // Only make this menu the current menu if requested, or if no @@ -429,13 +429,13 @@ void MenuManager::set_current_menu(Menu* menu) m_current_search.clear(); Menu* previous_current_menu = m_current_menu; - m_current_menu = menu->make_weak_ptr(); + m_current_menu = menu; auto& wm = WindowManager::the(); if (!previous_current_menu) { // When opening the first menu, store the current active input window if (auto* active_input = wm.active_input_window()) - m_previous_input_window = active_input->make_weak_ptr(); + m_previous_input_window = *active_input; else m_previous_input_window = nullptr; } @@ -457,7 +457,7 @@ Gfx::IntRect MenuManager::menubar_rect() const void MenuManager::set_current_menubar(MenuBar* menubar) { if (menubar) - m_current_menubar = menubar->make_weak_ptr(); + m_current_menubar = *menubar; else m_current_menubar = nullptr; #ifdef DEBUG_MENUS @@ -485,7 +485,7 @@ void MenuManager::close_menubar(MenuBar& menubar) void MenuManager::set_system_menu(Menu& menu) { - m_system_menu = menu.make_weak_ptr(); + m_system_menu = menu; set_current_menubar(m_current_menubar); } diff --git a/Services/WindowServer/Window.cpp b/Services/WindowServer/Window.cpp index aca43ccb56c..6c1c5275f3f 100644 --- a/Services/WindowServer/Window.cpp +++ b/Services/WindowServer/Window.cpp @@ -659,18 +659,18 @@ void Window::recalculate_rect() void Window::add_child_window(Window& child_window) { - m_child_windows.append(child_window.make_weak_ptr()); + m_child_windows.append(child_window); } void Window::add_accessory_window(Window& accessory_window) { - m_accessory_windows.append(accessory_window.make_weak_ptr()); + m_accessory_windows.append(accessory_window); } void Window::set_parent_window(Window& parent_window) { ASSERT(!m_parent_window); - m_parent_window = parent_window.make_weak_ptr(); + m_parent_window = parent_window; if (m_accessory) parent_window.add_accessory_window(*this); else diff --git a/Services/WindowServer/WindowManager.cpp b/Services/WindowServer/WindowManager.cpp index 9dfd1dcaf9d..bd566d585ab 100644 --- a/Services/WindowServer/WindowManager.cpp +++ b/Services/WindowServer/WindowManager.cpp @@ -444,7 +444,7 @@ void WindowManager::start_window_move(Window& window, const MouseEvent& event) dbg() << "[WM] Begin moving Window{" << &window << "}"; #endif move_to_front_and_make_active(window); - m_move_window = window.make_weak_ptr(); + m_move_window = window; m_move_window->set_default_positioned(false); m_move_origin = event.position(); m_move_window_origin = window.position(); @@ -478,7 +478,7 @@ void WindowManager::start_window_resize(Window& window, const Gfx::IntPoint& pos dbg() << "[WM] Begin resizing Window{" << &window << "}"; #endif m_resizing_mouse_button = button; - m_resize_window = window.make_weak_ptr(); + m_resize_window = window; m_resize_origin = position; m_resize_window_original_rect = window.rect(); @@ -739,7 +739,7 @@ bool WindowManager::process_ongoing_drag(MouseEvent& event, Window*& hovered_win void WindowManager::set_cursor_tracking_button(Button* button) { - m_cursor_tracking_button = button ? button->make_weak_ptr() : nullptr; + m_cursor_tracking_button = button; } auto WindowManager::DoubleClickInfo::metadata_for_button(MouseButton button) const -> const ClickMetadata& @@ -811,7 +811,7 @@ void WindowManager::start_menu_doubleclick(Window& window, const MouseEvent& eve #if defined(DOUBLECLICK_DEBUG) dbg() << "Initial mousedown on window " << &window << " for menu (previous was " << m_double_click_info.m_clicked_window << ')'; #endif - m_double_click_info.m_clicked_window = window.make_weak_ptr(); + m_double_click_info.m_clicked_window = window; m_double_click_info.reset(); } @@ -844,7 +844,7 @@ void WindowManager::process_event_for_doubleclick(Window& window, MouseEvent& ev #if defined(DOUBLECLICK_DEBUG) dbg() << "Initial mouseup on window " << &window << " (previous was " << m_double_click_info.m_clicked_window << ')'; #endif - m_double_click_info.m_clicked_window = window.make_weak_ptr(); + m_double_click_info.m_clicked_window = window; m_double_click_info.reset(); } @@ -994,7 +994,7 @@ void WindowManager::process_mouse_event(MouseEvent& event, Window*& hovered_wind auto translated_event = event.translated(-window.position()); deliver_mouse_event(window, translated_event); if (event.type() == Event::MouseDown) { - m_active_input_tracking_window = window.make_weak_ptr(); + m_active_input_tracking_window = window; } } return; @@ -1135,7 +1135,7 @@ void WindowManager::set_highlight_window(Window* window) return; if (auto* previous_highlight_window = m_highlight_window.ptr()) previous_highlight_window->invalidate(); - m_highlight_window = window ? window->make_weak_ptr() : nullptr; + m_highlight_window = window; if (m_highlight_window) m_highlight_window->invalidate(); } @@ -1179,7 +1179,7 @@ Window* WindowManager::set_active_input_window(Window* window) Core::EventLoop::current().post_event(*previous_input_window, make(Event::WindowInputLeft)); if (window) { - m_active_input_window = window->make_weak_ptr(); + m_active_input_window = *window; Core::EventLoop::current().post_event(*window, make(Event::WindowInputEntered)); } else { m_active_input_window = nullptr; @@ -1230,7 +1230,7 @@ void WindowManager::set_active_window(Window* window, bool make_input) } if (window) { - m_active_window = window->make_weak_ptr(); + m_active_window = *window; active_client = m_active_window->client(); Core::EventLoop::current().post_event(*m_active_window, make(Event::WindowActivated)); m_active_window->invalidate(); @@ -1260,7 +1260,7 @@ void WindowManager::set_hovered_window(Window* window) if (m_hovered_window) Core::EventLoop::current().post_event(*m_hovered_window, make(Event::WindowLeft)); - m_hovered_window = window ? window->make_weak_ptr() : nullptr; + m_hovered_window = window; if (m_hovered_window) Core::EventLoop::current().post_event(*m_hovered_window, make(Event::WindowEntered)); @@ -1314,12 +1314,12 @@ const Cursor& WindowManager::active_cursor() const void WindowManager::set_hovered_button(Button* button) { - m_hovered_button = button ? button->make_weak_ptr() : nullptr; + m_hovered_button = button; } void WindowManager::set_resize_candidate(Window& window, ResizeDirection direction) { - m_resize_candidate = window.make_weak_ptr(); + m_resize_candidate = window; m_resize_direction = direction; } @@ -1358,7 +1358,7 @@ Gfx::IntRect WindowManager::maximized_window_rect(const Window& window) const void WindowManager::start_dnd_drag(ClientConnection& client, const String& text, Gfx::Bitmap* bitmap, const Core::MimeData& mime_data) { ASSERT(!m_dnd_client); - m_dnd_client = client.make_weak_ptr(); + m_dnd_client = client; m_dnd_text = text; m_dnd_bitmap = bitmap; m_dnd_mime_data = mime_data; diff --git a/Services/WindowServer/WindowSwitcher.cpp b/Services/WindowServer/WindowSwitcher.cpp index e75e0ac93c2..803887c657d 100644 --- a/Services/WindowServer/WindowSwitcher.cpp +++ b/Services/WindowServer/WindowSwitcher.cpp @@ -230,7 +230,7 @@ void WindowSwitcher::refresh() longest_title_width = max(longest_title_width, wm.font().width(window.title())); if (selected_window == &window) m_selected_index = m_windows.size(); - m_windows.append(window.make_weak_ptr()); + m_windows.append(window); return IterationDecision::Continue; }, true);