From f375a959155efe7137aee60bb360207b09cf62a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=89=AC=E5=AE=81?= Date: Wed, 3 Jan 2024 11:37:49 +0800 Subject: [PATCH 1/3] Fix scoped_refptr leave ptr_ uninit when move construct by nullptr --- src/butil/memory/ref_counted.h | 8 +++----- test/ref_counted_unittest.cc | 13 +++++++++++++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/butil/memory/ref_counted.h b/src/butil/memory/ref_counted.h index 8fbb5d0ddd..e910004a03 100644 --- a/src/butil/memory/ref_counted.h +++ b/src/butil/memory/ref_counted.h @@ -285,21 +285,19 @@ class scoped_refptr { ptr_->AddRef(); } - scoped_refptr(scoped_refptr&& r) noexcept { + scoped_refptr(scoped_refptr&& r) noexcept : ptr_(r.get()) { if (r.ptr_){ - ptr_ = r.ptr_; r.ptr_ = nullptr; } } template - scoped_refptr(scoped_refptr&& r) noexcept { + scoped_refptr(scoped_refptr&& r) noexcept : ptr_(r.get()) { if (r.ptr_){ - ptr_ = r.ptr_; r.ptr_ = nullptr; } } - + ~scoped_refptr() { if (ptr_) ptr_->Release(); diff --git a/test/ref_counted_unittest.cc b/test/ref_counted_unittest.cc index 8de760026e..7415ba3e84 100644 --- a/test/ref_counted_unittest.cc +++ b/test/ref_counted_unittest.cc @@ -82,3 +82,16 @@ TEST(RefCountedUnitTest, ScopedRefPtrBooleanOperations) { EXPECT_NE(raw_p, p2); EXPECT_EQ(p1, p2); } + +TEST(RefCountedUnitTest, ScopedRefPtrMoveCtor) +{ + scoped_refptr p1 = new SelfAssign; + EXPECT_TRUE(p1); + + scoped_refptr p2(std::move(p1)); + EXPECT_TRUE(p2); + EXPECT_FALSE(p1); + + scoped_refptr p3(std::move(p1)); + EXPECT_FALSE(p3); +} From cc1edc423101f60ac50ff487a2114a69275db7e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=89=AC=E5=AE=81?= Date: Wed, 3 Jan 2024 13:24:10 +0800 Subject: [PATCH 2/3] change get() to ptr_ --- src/butil/memory/ref_counted.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/butil/memory/ref_counted.h b/src/butil/memory/ref_counted.h index e910004a03..dc0c6ee55e 100644 --- a/src/butil/memory/ref_counted.h +++ b/src/butil/memory/ref_counted.h @@ -285,14 +285,14 @@ class scoped_refptr { ptr_->AddRef(); } - scoped_refptr(scoped_refptr&& r) noexcept : ptr_(r.get()) { + scoped_refptr(scoped_refptr&& r) noexcept : ptr_(r.ptr_) { if (r.ptr_){ r.ptr_ = nullptr; } } template - scoped_refptr(scoped_refptr&& r) noexcept : ptr_(r.get()) { + scoped_refptr(scoped_refptr&& r) noexcept : ptr_(r.ptr_) { if (r.ptr_){ r.ptr_ = nullptr; } From 8322d715c906082a1dc1b7229f96099ba87f072c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=89=AC=E5=AE=81?= Date: Wed, 3 Jan 2024 14:10:49 +0800 Subject: [PATCH 3/3] remove if to make operation close --- src/butil/memory/ref_counted.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/butil/memory/ref_counted.h b/src/butil/memory/ref_counted.h index dc0c6ee55e..fb8bc72d0a 100644 --- a/src/butil/memory/ref_counted.h +++ b/src/butil/memory/ref_counted.h @@ -285,17 +285,15 @@ class scoped_refptr { ptr_->AddRef(); } - scoped_refptr(scoped_refptr&& r) noexcept : ptr_(r.ptr_) { - if (r.ptr_){ - r.ptr_ = nullptr; - } + scoped_refptr(scoped_refptr&& r) noexcept { + ptr_ = r.ptr_; + r.ptr_ = nullptr; } template - scoped_refptr(scoped_refptr&& r) noexcept : ptr_(r.ptr_) { - if (r.ptr_){ - r.ptr_ = nullptr; - } + scoped_refptr(scoped_refptr&& r) noexcept { + ptr_ = r.ptr_; + r.ptr_ = nullptr; } ~scoped_refptr() {