diff options
author | sv-denisov <sv-denisov@yandex-team.com> | 2023-01-14 15:13:39 +0300 |
---|---|---|
committer | sv-denisov <sv-denisov@yandex-team.com> | 2023-01-14 15:13:39 +0300 |
commit | 3657be5988251fc9074ba5b86b62bfa985ff4643 (patch) | |
tree | 2f9eca1c7158569dec9e2397c39b1a3731a7ee1c | |
parent | 380ce27d41b76ca1640e48b7271681b3719d6be0 (diff) | |
download | ydb-3657be5988251fc9074ba5b86b62bfa985ff4643.tar.gz |
TSharedPtr::As()&: leakage fix
По следам https://a.yandex-team.ru/review/2996409/details
Исправлены случаи при неудачных кастах:
1. контрольный блок дёргается даже при указании на nullptr (нехорошо для атомиков)
2. если исходный шаред поинтер разрушится раньше, чем полученный nullptr (например, при возврате из функции), то ресурс утечёт
-rw-r--r-- | util/generic/ptr.h | 21 | ||||
-rw-r--r-- | util/generic/ptr_ut.cpp | 44 |
2 files changed, 59 insertions, 6 deletions
diff --git a/util/generic/ptr.h b/util/generic/ptr.h index b35d54d780..cc2e3c0f51 100644 --- a/util/generic/ptr.h +++ b/util/generic/ptr.h @@ -900,18 +900,27 @@ public: [[nodiscard]] inline TSharedPtr<TT, C, D> As() & noexcept { static_assert(std::has_virtual_destructor<TT>(), "Type should have a virtual dtor"); static_assert(std::is_base_of<T, TT>(), "When downcasting from T to TT, T should be a parent of TT"); - Ref(); - return TSharedPtr<TT, C, D>(dynamic_cast<TT*>(T_), C_); + if (const auto ttPtr = dynamic_cast<TT*>(T_)) { + TSharedPtr<TT, C, D> ttSharedPtr(ttPtr, C_); + ttSharedPtr.Ref(); + return ttSharedPtr; + } else { + return TSharedPtr<TT, C, D>{}; + } } template <class TT> [[nodiscard]] inline TSharedPtr<TT, C, D> As() && noexcept { static_assert(std::has_virtual_destructor<TT>(), "Type should have a virtual dtor"); static_assert(std::is_base_of<T, TT>(), "When downcasting from T to TT, T should be a parent of TT"); - auto resultPtr = TSharedPtr<TT, C, D>(dynamic_cast<TT*>(T_), C_); - T_ = nullptr; - C_ = nullptr; - return resultPtr; + if (const auto ttPtr = dynamic_cast<TT*>(T_)) { + TSharedPtr<TT, C, D> ttSharedPtr(ttPtr, C_); + T_ = nullptr; + C_ = nullptr; + return ttSharedPtr; + } else { + return TSharedPtr<TT, C, D>{}; + } } #ifdef __cpp_impl_three_way_comparison diff --git a/util/generic/ptr_ut.cpp b/util/generic/ptr_ut.cpp index 6e028725b4..5f0fd2d470 100644 --- a/util/generic/ptr_ut.cpp +++ b/util/generic/ptr_ut.cpp @@ -850,6 +850,11 @@ public: using TVirtualProbe::TVirtualProbe; }; +class TDerivedProbeSibling: public TVirtualProbe { +public: + using TVirtualProbe::TVirtualProbe; +}; + void TPointerTest::TestSharedPtrDowncast() { { NTesting::TProbeState probeState = {}; @@ -889,4 +894,43 @@ void TPointerTest::TestSharedPtrDowncast() { UNIT_ASSERT_VALUES_EQUAL(probeState.Destructors, 1); } + { + NTesting::TProbeState probeState = {}; + + { + TSimpleSharedPtr<TVirtualProbe> base = MakeSimpleShared<TDerivedProbe>(&probeState); + UNIT_ASSERT_VALUES_EQUAL(probeState.Constructors, 1); + + { + auto derivedSibling = base.As<TDerivedProbeSibling>(); + UNIT_ASSERT_VALUES_EQUAL(probeState.Constructors, 1); + + UNIT_ASSERT_VALUES_EQUAL(derivedSibling.Get(), nullptr); + UNIT_ASSERT_VALUES_UNEQUAL(base.ReferenceCounter(), derivedSibling.ReferenceCounter()); + + UNIT_ASSERT_VALUES_EQUAL(base.RefCount(), 1l); + UNIT_ASSERT_VALUES_EQUAL(derivedSibling.RefCount(), 0l); + } + + UNIT_ASSERT_VALUES_EQUAL(probeState.Destructors, 0); + } + + UNIT_ASSERT_VALUES_EQUAL(probeState.Destructors, 1); + } + { + NTesting::TProbeState probeState = {}; + + { + TSimpleSharedPtr<TVirtualProbe> base = MakeSimpleShared<TDerivedProbe>(&probeState); + UNIT_ASSERT_VALUES_EQUAL(probeState.Constructors, 1); + + auto derived = std::move(base).As<TDerivedProbeSibling>(); + UNIT_ASSERT_VALUES_EQUAL(derived.Get(), nullptr); + UNIT_ASSERT_VALUES_EQUAL(probeState.Constructors, 1); + UNIT_ASSERT_VALUES_EQUAL(probeState.CopyConstructors, 0); + UNIT_ASSERT_VALUES_EQUAL(probeState.Destructors, 0); + } + + UNIT_ASSERT_VALUES_EQUAL(probeState.Destructors, 1); + } } |