diff --git a/stl/inc/regex b/stl/inc/regex index 02badc1ed90..d17ea56bb56 100644 --- a/stl/inc/regex +++ b/stl/inc/regex @@ -2516,8 +2516,8 @@ public: _MyRe = nullptr; #if _ITERATOR_DEBUG_LEVEL == 2 - this->_Orphan_me_v2(); -#endif // _ITERATOR_DEBUG_LEVEL + this->_Adopt(nullptr); +#endif // _ITERATOR_DEBUG_LEVEL == 2 return *this; } diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 28bbd804cbb..b94808a5307 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1120,12 +1120,12 @@ public: _Container_proxy* _Myproxy = nullptr; private: - _CONSTEXPR20 void _Orphan_all_unlocked() noexcept; + _CONSTEXPR20 void _Orphan_all_unlocked_v3() noexcept; _CONSTEXPR20 void _Swap_proxy_and_iterators_unlocked(_Container_base12&) noexcept; - void _Orphan_all_locked() noexcept { + void _Orphan_all_locked_v3() noexcept { _Lockit _Lock(_LOCK_DEBUG); - _Orphan_all_unlocked(); + _Orphan_all_unlocked_v3(); } void _Swap_proxy_and_iterators_locked(_Container_base12& _Right) noexcept { @@ -1143,56 +1143,43 @@ public: } _CONSTEXPR20 _Iterator_base12& operator=(const _Iterator_base12& _Right) noexcept { - if (_Myproxy != _Right._Myproxy) { - if (_Right._Myproxy) { - _Adopt(_Right._Myproxy->_Mycont); - } else { // becoming invalid, disown current parent #if _ITERATOR_DEBUG_LEVEL == 2 - _Orphan_me_v2(); -#else // _ITERATOR_DEBUG_LEVEL == 2 - _Myproxy = nullptr; -#endif // _ITERATOR_DEBUG_LEVEL == 2 - } +#if _HAS_CXX20 + if (_STD is_constant_evaluated()) { + _Assign_unlocked(_Right); + } else +#endif // _HAS_CXX20 + { + _Assign_locked(_Right); } +#else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 ^^^ / vvv _ITERATOR_DEBUG_LEVEL != 2 vvv + _Myproxy = _Right._Myproxy; +#endif // _ITERATOR_DEBUG_LEVEL != 2 return *this; } #if _ITERATOR_DEBUG_LEVEL == 2 _CONSTEXPR20 ~_Iterator_base12() noexcept { - _Orphan_me_v2(); - } - - _CONSTEXPR20 void _Adopt(const _Container_base12* _Parent) noexcept { - if (_Parent) { // have a parent, do adoption - _Container_proxy* _Parent_proxy = _Parent->_Myproxy; - if (_Myproxy != _Parent_proxy) { // change parentage #if _HAS_CXX20 - if (_STD is_constant_evaluated()) { - _Adopt_unlocked(_Parent_proxy); - } else + if (_STD is_constant_evaluated()) { + _Orphan_me_unlocked_v3(); + } else #endif // _HAS_CXX20 - { - _Adopt_locked(_Parent_proxy); - } - } - } else { // no future parent, just disown current parent - _Orphan_me_v2(); + { + _Orphan_me_locked_v3(); } } - _CONSTEXPR20 void _Orphan_me_v2() noexcept { - if (_Myproxy) { // adopted, remove self from list + _CONSTEXPR20 void _Adopt(const _Container_base12* _Parent) noexcept { #if _HAS_CXX20 - if (_STD is_constant_evaluated()) { - _Orphan_me_unlocked(); - } else + if (_STD is_constant_evaluated()) { + _Adopt_unlocked(_Parent); + } else #endif // _HAS_CXX20 - { - _Orphan_me_locked(); - } + { + _Adopt_locked(_Parent); } } - #else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 ^^^ / vvv _ITERATOR_DEBUG_LEVEL != 2 vvv _CONSTEXPR20 void _Adopt(const _Container_base12* _Parent) noexcept { if (_Parent) { // have a parent, do adoption @@ -1214,21 +1201,51 @@ public: #if _ITERATOR_DEBUG_LEVEL == 2 private: - _CONSTEXPR20 void _Adopt_unlocked(_Container_proxy* _Parent_proxy) noexcept { - if (_Myproxy) { // adopted, remove self from list - _Orphan_me_unlocked(); + _CONSTEXPR20 void _Assign_unlocked(const _Iterator_base12& _Right) noexcept { + if (_Myproxy == _Right._Myproxy) { + return; + } + + if (_Right._Myproxy) { + _Adopt_unlocked(_Right._Myproxy->_Mycont); + } else { // becoming invalid, disown current parent + _Orphan_me_unlocked_v3(); + } + } + + void _Assign_locked(const _Iterator_base12& _Right) noexcept { + _Lockit _Lock(_LOCK_DEBUG); + _Assign_unlocked(_Right); + } + + _CONSTEXPR20 void _Adopt_unlocked(const _Container_base12* _Parent) noexcept { + if (!_Parent) { + _Orphan_me_unlocked_v3(); + return; + } + + _Container_proxy* _Parent_proxy = _Parent->_Myproxy; + if (_Myproxy != _Parent_proxy) { // change parentage + if (_Myproxy) { // adopted, remove self from list + _Orphan_me_unlocked_v3(); + } + _Mynextiter = _Parent_proxy->_Myfirstiter; + _Parent_proxy->_Myfirstiter = this; + _Myproxy = _Parent_proxy; } - _Mynextiter = _Parent_proxy->_Myfirstiter; - _Parent_proxy->_Myfirstiter = this; - _Myproxy = _Parent_proxy; } - void _Adopt_locked(_Container_proxy* _Parent_proxy) noexcept { + void _Adopt_locked(const _Container_base12* _Parent) noexcept { _Lockit _Lock(_LOCK_DEBUG); - _Adopt_unlocked(_Parent_proxy); + _Adopt_unlocked(_Parent); } - _CONSTEXPR20 void _Orphan_me_unlocked() noexcept { + _CONSTEXPR20 void _Orphan_me_unlocked_v3() noexcept { + if (!_Myproxy) { // already orphaned + return; + } + + // adopted, remove self from list _Iterator_base12** _Pnext = &_Myproxy->_Myfirstiter; while (*_Pnext && *_Pnext != this) { const auto _Temp = *_Pnext; // TRANSITION, VSO-1269037 @@ -1240,15 +1257,20 @@ private: _Myproxy = nullptr; } - void _Orphan_me_locked() noexcept { + void _Orphan_me_locked_v3() noexcept { _Lockit _Lock(_LOCK_DEBUG); - _Orphan_me_unlocked(); + _Orphan_me_unlocked_v3(); } #endif // _ITERATOR_DEBUG_LEVEL == 2 }; // MEMBER FUNCTIONS FOR _Container_base12 -_CONSTEXPR20 void _Container_base12::_Orphan_all_unlocked() noexcept { +_CONSTEXPR20 void _Container_base12::_Orphan_all_unlocked_v3() noexcept { + if (!_Myproxy) { // no proxy, already done + return; + } + + // proxy allocated, drain it for (auto& _Pnext = _Myproxy->_Myfirstiter; _Pnext; _Pnext = _Pnext->_Mynextiter) { // TRANSITION, VSO-1269037 _Pnext->_Myproxy = nullptr; } @@ -1257,15 +1279,13 @@ _CONSTEXPR20 void _Container_base12::_Orphan_all_unlocked() noexcept { _CONSTEXPR20 void _Container_base12::_Orphan_all() noexcept { #if _ITERATOR_DEBUG_LEVEL == 2 - if (_Myproxy) { // proxy allocated, drain it #if _HAS_CXX20 - if (_STD is_constant_evaluated()) { - _Orphan_all_unlocked(); - } else + if (_STD is_constant_evaluated()) { + _Orphan_all_unlocked_v3(); + } else #endif // _HAS_CXX20 - { - _Orphan_all_locked(); - } + { + _Orphan_all_locked_v3(); } #endif // _ITERATOR_DEBUG_LEVEL == 2 } diff --git a/tests/std/test.lst b/tests/std/test.lst index a9aaf5b9b00..3fb435eca19 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -177,6 +177,7 @@ tests\GH_001541_case_sensitive_boolalpha tests\GH_001638_dllexport_derived_classes tests\GH_001850_clog_tied_to_cout tests\GH_002039_byte_is_not_trivially_swappable +tests\GH_002058_debug_iterator_race tests\LWG2597_complex_branch_cut tests\LWG3018_shared_ptr_function tests\P0019R8_atomic_ref diff --git a/tests/std/tests/GH_002058_debug_iterator_race/env.lst b/tests/std/tests/GH_002058_debug_iterator_race/env.lst new file mode 100644 index 00000000000..e970fe46ae6 --- /dev/null +++ b/tests/std/tests/GH_002058_debug_iterator_race/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\native_matrix.lst diff --git a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp new file mode 100644 index 00000000000..7dedf81b9b5 --- /dev/null +++ b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include + +using namespace std; + +// Concurrently destroy and invalidate iterators +template +void test_concurrent_destruction() { + Container c; + c.insert(c.begin(), 0); + + vector iters(1000, c.begin()); + { + auto destroyIters = async(launch::async, [&]() { iters.clear(); }); + + auto invalidateIters = async(launch::async, [&]() { c.clear(); }); + } +} + +// Concurrently create and invalidate iterators +template +void test_concurrent_creation() { + Container c; + c.insert(c.begin(), 0); + + vector iters; + iters.reserve(1000); + + const auto iter = c.begin(); + { + auto copyIters = async(launch::async, [&]() { + for (int i = 0; i < 1000; ++i) { + iters.push_back(iter); + }; + }); + + auto invalidateIters = async(launch::async, [&]() { c.clear(); }); + } +} + +int main() { + test_concurrent_destruction>(); + test_concurrent_destruction>(); + test_concurrent_destruction>(); + + test_concurrent_creation>(); + test_concurrent_creation>(); + test_concurrent_creation>(); +}