From 4ed7ec5e93794ca57000e3ff69f0b73e5f2edbd6 Mon Sep 17 00:00:00 2001 From: apio Date: Sun, 30 Jul 2023 11:32:46 +0200 Subject: [PATCH] libluna: Store SharedPtr's ref count in the object itself --- kernel/src/arch/x86_64/disk/ATA.h | 4 +- kernel/src/fs/Pipe.h | 2 +- kernel/src/fs/VFS.h | 4 +- kernel/src/fs/devices/Device.h | 3 +- libluna/include/luna/SharedPtr.h | 89 ++++++++++++------------------- libos/include/os/Directory.h | 2 +- libos/include/os/File.h | 2 +- tests/CMakeLists.txt | 3 +- tests/libluna/TestSharedPtr.cpp | 61 +++++++++++++++++++++ 9 files changed, 107 insertions(+), 63 deletions(-) create mode 100644 tests/libluna/TestSharedPtr.cpp diff --git a/kernel/src/arch/x86_64/disk/ATA.h b/kernel/src/arch/x86_64/disk/ATA.h index fb8c9122..6b3e367d 100644 --- a/kernel/src/arch/x86_64/disk/ATA.h +++ b/kernel/src/arch/x86_64/disk/ATA.h @@ -131,7 +131,7 @@ namespace ATA static constexpr u16 END_OF_PRDT = (1 << 15); - class Drive + class Drive : public Shareable { public: Drive(Channel* channel, u8 drive_index, Badge); @@ -274,7 +274,7 @@ namespace ATA SharedPtr m_drives[2]; }; - class Controller + class Controller : public Shareable { public: static Result scan(); diff --git a/kernel/src/fs/Pipe.h b/kernel/src/fs/Pipe.h index e95a2424..e84368ef 100644 --- a/kernel/src/fs/Pipe.h +++ b/kernel/src/fs/Pipe.h @@ -6,7 +6,7 @@ class PipeInodeBase; class PipeReader; class PipeWriter; -class Pipe +class Pipe : public Shareable { public: static Result create(SharedPtr& rpipe, SharedPtr& wpipe); diff --git a/kernel/src/fs/VFS.h b/kernel/src/fs/VFS.h index f89eaa4d..7dcee9d5 100644 --- a/kernel/src/fs/VFS.h +++ b/kernel/src/fs/VFS.h @@ -21,7 +21,7 @@ namespace VFS class Inode; - class FileSystem + class FileSystem : public Shareable { public: virtual SharedPtr root_inode() const = 0; @@ -73,7 +73,7 @@ namespace VFS StaticString<128> name; }; - class Inode + class Inode : public Shareable { public: virtual Result ioctl(int, void*) diff --git a/kernel/src/fs/devices/Device.h b/kernel/src/fs/devices/Device.h index daa43425..e72a9e24 100644 --- a/kernel/src/fs/devices/Device.h +++ b/kernel/src/fs/devices/Device.h @@ -1,8 +1,9 @@ #pragma once #include "Log.h" #include +#include -class Device +class Device : public Shareable { public: virtual Result read(u8* buf, usize offset, usize length) const = 0; diff --git a/libluna/include/luna/SharedPtr.h b/libluna/include/luna/SharedPtr.h index a71f372c..a369e75c 100644 --- a/libluna/include/luna/SharedPtr.h +++ b/libluna/include/luna/SharedPtr.h @@ -5,89 +5,76 @@ #include #include #include +#include -namespace __detail +template class SharedPtr; +template SharedPtr adopt_shared(T*); + +struct Shareable { - struct RefCount + void ref() { - void ref() - { - m_ref_count++; - } + m_ref_count++; + } - bool unref() - { - m_ref_count--; - return m_ref_count == 0; - } + bool unref() + { + m_ref_count--; + return m_ref_count == 0; + } - private: - Atomic m_ref_count { 1 }; - }; -} + Atomic m_ref_count { 1 }; +}; template class SharedPtr { - using RefCount = __detail::RefCount; - public: SharedPtr() { m_ptr = nullptr; - m_ref_count = nullptr; } - SharedPtr(T* ptr, RefCount* ref_count) : m_ptr(ptr), m_ref_count(ref_count) + SharedPtr(T* ptr) : m_ptr(ptr) { } - SharedPtr(const SharedPtr& other) : m_ptr(other.m_ptr), m_ref_count(other.m_ref_count) + SharedPtr(const SharedPtr& other) : m_ptr(other.m_ptr) { - if (m_ref_count) m_ref_count->ref(); + if (m_ptr) shareable()->ref(); } - SharedPtr(SharedPtr&& other) : m_ptr(other.m_ptr), m_ref_count(other.m_ref_count) + SharedPtr(SharedPtr&& other) : m_ptr(other.m_ptr) { other.m_ptr = nullptr; - other.m_ref_count = nullptr; } template operator SharedPtr() { - if (m_ref_count) m_ref_count->ref(); - return { (Tp*)m_ptr, m_ref_count }; + if (m_ptr) shareable()->ref(); + return { (Tp*)m_ptr }; } ~SharedPtr() { - if (m_ref_count && m_ref_count->unref()) - { - delete m_ref_count; - delete m_ptr; - } + if (m_ptr && shareable()->unref()) { delete m_ptr; } } SharedPtr& operator=(const SharedPtr& other) { if (&other == this) return *this; - if (m_ref_count && m_ref_count->unref()) - { - delete m_ref_count; - delete m_ptr; - } + if (m_ptr && shareable()->unref()) { delete m_ptr; } m_ptr = other.m_ptr; - m_ref_count = other.m_ref_count; - if (m_ref_count) m_ref_count->ref(); + if (m_ptr) shareable()->ref(); return *this; } bool operator==(const SharedPtr& other) { - return m_ptr == other.m_ptr && m_ref_count == other.m_ref_count; + return m_ptr == other.m_ptr; } T* ptr() const @@ -112,21 +99,17 @@ template class SharedPtr private: T* m_ptr; - RefCount* m_ref_count; + + Shareable* shareable() + { + static_assert(IsBaseOf); + return (Shareable*)m_ptr; + } }; -// NOTE: ptr is deleted if any of the adopt_shared* functions fail to construct a SharedPtr. -template Result> adopt_shared(T* ptr) +template SharedPtr adopt_shared(T* ptr) { - using RefCount = __detail::RefCount; - - auto guard = make_scope_guard([ptr] { delete ptr; }); - - RefCount* const ref_count = TRY(make()); - - guard.deactivate(); - - return SharedPtr { ptr, ref_count }; + return SharedPtr { ptr }; } template Result> make_shared(Args... args) @@ -142,12 +125,10 @@ template Result> adopt_shared_if_nonnull(T* ptr) return err(ENOMEM); } -template Result> adopt_shared_from_owned(OwnedPtr&& other) +template SharedPtr adopt_shared_from_owned(OwnedPtr&& other) { T* ptr = other.m_ptr; other.m_ptr = nullptr; - const SharedPtr shared_ptr = TRY(adopt_shared(ptr)); - - return shared_ptr; + return SharedPtr { ptr }; } diff --git a/libos/include/os/Directory.h b/libos/include/os/Directory.h index bc8a4c20..a7552b10 100644 --- a/libos/include/os/Directory.h +++ b/libos/include/os/Directory.h @@ -19,7 +19,7 @@ namespace os /** * @brief An object-oriented directory handle, which is closed when all references to it go out of scope. */ - class Directory + class Directory : public Shareable { public: /** diff --git a/libos/include/os/File.h b/libos/include/os/File.h index 1e280dce..a017d6b3 100644 --- a/libos/include/os/File.h +++ b/libos/include/os/File.h @@ -22,7 +22,7 @@ namespace os /** * @brief An object-oriented file handle, which is closed when all references to it go out of scope. */ - class File + class File : public Shareable { public: /** diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index d090faea..72d0856a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -5,7 +5,7 @@ target_include_directories(test PUBLIC ${LUNA_BASE}/usr/include) function(luna_test SOURCE_FILE APP_NAME) add_executable(${APP_NAME} ${SOURCE_FILE}) - target_compile_options(${APP_NAME} PRIVATE -Os ${COMMON_FLAGS} -Wno-write-strings) + target_compile_options(${APP_NAME} PRIVATE -O0 ${COMMON_FLAGS} -fno-threadsafe-statics -Wno-write-strings) add_dependencies(${APP_NAME} libc) target_include_directories(${APP_NAME} PRIVATE ${LUNA_BASE}/usr/include) target_link_libraries(${APP_NAME} PRIVATE test os) @@ -19,6 +19,7 @@ luna_test(libluna/TestUtf8.cpp TestUtf8) luna_test(libluna/TestFormat.cpp TestFormat) luna_test(libluna/TestHashTable.cpp TestHashTable) luna_test(libluna/TestCPath.cpp TestCPath) +luna_test(libluna/TestSharedPtr.cpp TestSharedPtr) luna_test(libc/TestScanf.cpp TestScanf) luna_test(libc/TestString.cpp TestString) luna_test(libc/TestEnv.cpp TestEnv) diff --git a/tests/libluna/TestSharedPtr.cpp b/tests/libluna/TestSharedPtr.cpp new file mode 100644 index 00000000..c50714fe --- /dev/null +++ b/tests/libluna/TestSharedPtr.cpp @@ -0,0 +1,61 @@ +#include +#include + +class SampleClass : public Shareable +{ + public: + SampleClass(int* ptr, int value) : m_ptr(ptr), m_value(value) + { + } + + ~SampleClass() + { + if (m_ptr) *m_ptr = m_value; + } + + private: + int* m_ptr; + int m_value; +}; + +TestResult test_shared_ptr_destroyed_on_scope_exit() +{ + static int value = 0; + + { + SharedPtr sample = TRY(make_shared(&value, 1)); + } + + validate(value == 1); + + test_success; +} + +TestResult test_shared_ptr_preserved_if_at_least_one_copy_exists() +{ + static int value = 0; + static SharedPtr ptr = {}; + + { + SharedPtr sample = TRY(make_shared(&value, 1)); + ptr = sample; + } + + validate(value == 0); + + ptr = {}; + + validate(value == 1); + + test_success; +} + +Result test_main() +{ + test_prelude; + + run_test(test_shared_ptr_destroyed_on_scope_exit); + run_test(test_shared_ptr_preserved_if_at_least_one_copy_exists); + + return {}; +}