From df30d2a236bff2f2fce4e65330ea935e09015f74 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 14 Jul 2021 15:57:44 +0200 Subject: [PATCH 01/15] Fix race condition in iterator debug machinery Fixes #2058 --- stl/inc/xmemory | 68 +++++++++---------- tests/std/test.lst | 1 + .../GH_002058_debug_iterator_race/env.lst | 4 ++ .../GH_002058_debug_iterator_race/test.cpp | 51 ++++++++++++++ 4 files changed, 90 insertions(+), 34 deletions(-) create mode 100644 tests/std/tests/GH_002058_debug_iterator_race/env.lst create mode 100644 tests/std/tests/GH_002058_debug_iterator_race/test.cpp diff --git a/stl/inc/xmemory b/stl/inc/xmemory index bc1be142db..8183d8b2d3 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1132,33 +1132,24 @@ public: } _CONSTEXPR20_CONTAINER 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 #ifdef __cpp_lib_constexpr_dynamic_alloc - if (_STD is_constant_evaluated()) { - _Adopt_unlocked(_Parent_proxy); - } else + if (_STD is_constant_evaluated()) { + _Adopt_unlocked(_Parent); + } else #endif // __cpp_lib_constexpr_dynamic_alloc - { - _Adopt_locked(_Parent_proxy); - } - } - } else { // no future parent, just disown current parent - _Orphan_me_v2(); + { + _Adopt_locked(_Parent); } } _CONSTEXPR20_CONTAINER void _Orphan_me_v2() noexcept { - if (_Myproxy) { // adopted, remove self from list #ifdef __cpp_lib_constexpr_dynamic_alloc - if (_STD is_constant_evaluated()) { - _Orphan_me_unlocked(); - } else + if (_STD is_constant_evaluated()) { + _Orphan_me_unlocked(); + } else #endif // __cpp_lib_constexpr_dynamic_alloc - { - _Orphan_me_locked(); - } + { + _Orphan_me_locked(); } } @@ -1183,30 +1174,39 @@ public: #if _ITERATOR_DEBUG_LEVEL == 2 private: - _CONSTEXPR20_CONTAINER void _Adopt_unlocked(_Container_proxy* _Parent_proxy) noexcept { - if (_Myproxy) { // adopted, remove self from list + _CONSTEXPR20_CONTAINER void _Adopt_unlocked(const _Container_base12* _Parent) noexcept { + if (_Parent) { + _Container_proxy* _Parent_proxy = _Parent->_Myproxy; + if (_Myproxy != _Parent_proxy) { // change parentage + if (_Myproxy) { // adopted, remove self from list + _Orphan_me_unlocked(); + } + _Mynextiter = _Parent_proxy->_Myfirstiter; + _Parent_proxy->_Myfirstiter = this; + _Myproxy = _Parent_proxy; + } + } else { _Orphan_me_unlocked(); } - _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_CONTAINER void _Orphan_me_unlocked() noexcept { - _Iterator_base12** _Pnext = &_Myproxy->_Myfirstiter; - while (*_Pnext && *_Pnext != this) { - const auto _Temp = *_Pnext; // TRANSITION, VSO-1269037 - _Pnext = &_Temp->_Mynextiter; - } + if (_Myproxy) { // adopted, remove self from list + _Iterator_base12** _Pnext = &_Myproxy->_Myfirstiter; + while (*_Pnext && *_Pnext != this) { + const auto _Temp = *_Pnext; // TRANSITION, VSO-1269037 + _Pnext = &_Temp->_Mynextiter; + } - _STL_VERIFY(*_Pnext, "ITERATOR LIST CORRUPTED!"); - *_Pnext = _Mynextiter; - _Myproxy = nullptr; + _STL_VERIFY(*_Pnext, "ITERATOR LIST CORRUPTED!"); + *_Pnext = _Mynextiter; + _Myproxy = nullptr; + } } void _Orphan_me_locked() noexcept { diff --git a/tests/std/test.lst b/tests/std/test.lst index 38896e23e0..fcc33d5033 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -175,6 +175,7 @@ tests\GH_001411_core_headers tests\GH_001530_binomial_accuracy tests\GH_001541_case_sensitive_boolalpha tests\GH_001638_dllexport_derived_classes +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 0000000000..19f025bd0e --- /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 ..\usual_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 0000000000..7c2151e894 --- /dev/null +++ b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp @@ -0,0 +1,51 @@ +#include +#include +#include +#include +#include +#include + +using namespace std; + +// Concurrently destroy iterators 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 iterators and invalidate container +template +void test_concurrent_creation() { + container c; + c.insert(c.begin(), 0); + + vector iters; + iters.reserve(1000); + { + auto createIters = async(launch::async, [&]() { + for (int i = 0; i < 1000; ++i) { + iters.push_back(c.begin()); + }; + }); + + auto invalidateIters = async(launch::async, [&]() { c = container{}; }); + } +} + +int main() { + test_concurrent_destruction>(); + test_concurrent_destruction>(); + test_concurrent_destruction>(); + + test_concurrent_creation>(); + test_concurrent_creation>(); + test_concurrent_creation>(); +} From 3aefa0b160af6dc459beccd40aa146f43ab9d30b Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 14 Jul 2021 16:00:52 +0200 Subject: [PATCH 02/15] Also fix `_Orphan_all` --- stl/inc/xmemory | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 8183d8b2d3..7ed4efeb90 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1218,23 +1218,23 @@ private: // MEMBER FUNCTIONS FOR _Container_base12 _CONSTEXPR20_CONTAINER void _Container_base12::_Orphan_all_unlocked() noexcept { - for (auto& _Pnext = _Myproxy->_Myfirstiter; _Pnext; _Pnext = _Pnext->_Mynextiter) { // TRANSITION, VSO-1269037 - _Pnext->_Myproxy = nullptr; + if (_Myproxy) { // proxy allocated, drain it + for (auto& _Pnext = _Myproxy->_Myfirstiter; _Pnext; _Pnext = _Pnext->_Mynextiter) { // TRANSITION, VSO-1269037 + _Pnext->_Myproxy = nullptr; + } + _Myproxy->_Myfirstiter = nullptr; } - _Myproxy->_Myfirstiter = nullptr; } _CONSTEXPR20_CONTAINER void _Container_base12::_Orphan_all() noexcept { #if _ITERATOR_DEBUG_LEVEL == 2 - if (_Myproxy) { // proxy allocated, drain it #ifdef __cpp_lib_constexpr_dynamic_alloc - if (_STD is_constant_evaluated()) { - _Orphan_all_unlocked(); - } else + if (_STD is_constant_evaluated()) { + _Orphan_all_unlocked(); + } else #endif // __cpp_lib_constexpr_dynamic_alloc - { - _Orphan_all_locked(); - } + { + _Orphan_all_locked(); } #endif // _ITERATOR_DEBUG_LEVEL == 2 } From b0e7238228ee9697bdb30c30b864c7247da9dca7 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 15 Jul 2021 10:18:10 +0200 Subject: [PATCH 03/15] Address review comments Co-authored-by: Adam Bucior <35536269+AdamBucior@users.noreply.github.com> Co-authored-by: Casey Carter --- stl/inc/xmemory | 104 ++++++++++++------ .../GH_002058_debug_iterator_race/test.cpp | 6 +- 2 files changed, 75 insertions(+), 35 deletions(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 7ed4efeb90..c4cf2122a6 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1112,23 +1112,37 @@ public: } _CONSTEXPR20_CONTAINER _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 +#ifdef __cpp_lib_constexpr_dynamic_alloc + if (_STD is_constant_evaluated()) { + _Assign_unlocked(_Right); + } else +#endif // __cpp_lib_constexpr_dynamic_alloc + { + _Assign_locked(_Right); + } +#else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 ^^^ / vvv _ITERATOR_DEBUG_LEVEL != 2 vvv + if (_Myproxy != _Right._Myproxy) { + if (_Right._Myproxy && _Right._Myproxy->_Mycont) { + _Myproxy = _Right._Myproxy->_Mycont->_Myproxy; + } else { _Myproxy = nullptr; -#endif // _ITERATOR_DEBUG_LEVEL == 2 } } +#endif // _ITERATOR_DEBUG_LEVEL != 2 return *this; } #if _ITERATOR_DEBUG_LEVEL == 2 _CONSTEXPR20_CONTAINER ~_Iterator_base12() noexcept { - _Orphan_me_v2(); +#ifdef __cpp_lib_constexpr_dynamic_alloc + if (_STD is_constant_evaluated()) { + _Orphan_me_unlocked(); + } else +#endif // __cpp_lib_constexpr_dynamic_alloc + { + _Orphan_me_locked(); + } } _CONSTEXPR20_CONTAINER void _Adopt(const _Container_base12* _Parent) noexcept { @@ -1174,19 +1188,37 @@ public: #if _ITERATOR_DEBUG_LEVEL == 2 private: + _CONSTEXPR20_CONTAINER void _Assign_unlocked(const _Iterator_base12& _Right) noexcept { + if (_Myproxy == _Right._Myproxy) { + return; + } + + if (_Right._Myproxy && _Right._Myproxy->_Mycont) { + _Adopt_unlocked(_Right._Myproxy->_Mycont); + } else { // becoming invalid, disown current parent + _Orphan_me_unlocked(); + } + } + + void _Assign_locked(const _Iterator_base12& _Right) noexcept { + _Lockit _Lock(_LOCK_DEBUG); + _Assign_unlocked(_Right); + } + _CONSTEXPR20_CONTAINER void _Adopt_unlocked(const _Container_base12* _Parent) noexcept { - if (_Parent) { - _Container_proxy* _Parent_proxy = _Parent->_Myproxy; - if (_Myproxy != _Parent_proxy) { // change parentage - if (_Myproxy) { // adopted, remove self from list - _Orphan_me_unlocked(); - } - _Mynextiter = _Parent_proxy->_Myfirstiter; - _Parent_proxy->_Myfirstiter = this; - _Myproxy = _Parent_proxy; - } - } else { + if (!_Parent) { _Orphan_me_unlocked(); + return; + } + + _Container_proxy* _Parent_proxy = _Parent->_Myproxy; + if (_Myproxy != _Parent_proxy) { // change parentage + if (_Myproxy) { // adopted, remove self from list + _Orphan_me_unlocked(); + } + _Mynextiter = _Parent_proxy->_Myfirstiter; + _Parent_proxy->_Myfirstiter = this; + _Myproxy = _Parent_proxy; } } @@ -1196,17 +1228,20 @@ private: } _CONSTEXPR20_CONTAINER void _Orphan_me_unlocked() noexcept { - if (_Myproxy) { // adopted, remove self from list - _Iterator_base12** _Pnext = &_Myproxy->_Myfirstiter; - while (*_Pnext && *_Pnext != this) { - const auto _Temp = *_Pnext; // TRANSITION, VSO-1269037 - _Pnext = &_Temp->_Mynextiter; - } + if (!_Myproxy) { // Already orphaned + return; + } - _STL_VERIFY(*_Pnext, "ITERATOR LIST CORRUPTED!"); - *_Pnext = _Mynextiter; - _Myproxy = nullptr; + // adopted, remove self from list + _Iterator_base12** _Pnext = &_Myproxy->_Myfirstiter; + while (*_Pnext && *_Pnext != this) { + const auto _Temp = *_Pnext; // TRANSITION, VSO-1269037 + _Pnext = &_Temp->_Mynextiter; } + + _STL_VERIFY(*_Pnext, "ITERATOR LIST CORRUPTED!"); + *_Pnext = _Mynextiter; + _Myproxy = nullptr; } void _Orphan_me_locked() noexcept { @@ -1218,12 +1253,15 @@ private: // MEMBER FUNCTIONS FOR _Container_base12 _CONSTEXPR20_CONTAINER void _Container_base12::_Orphan_all_unlocked() noexcept { - if (_Myproxy) { // proxy allocated, drain it - for (auto& _Pnext = _Myproxy->_Myfirstiter; _Pnext; _Pnext = _Pnext->_Mynextiter) { // TRANSITION, VSO-1269037 - _Pnext->_Myproxy = nullptr; - } - _Myproxy->_Myfirstiter = nullptr; + 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; } + _Myproxy->_Myfirstiter = nullptr; } _CONSTEXPR20_CONTAINER void _Container_base12::_Orphan_all() noexcept { diff --git a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp index 7c2151e894..1cb137a0b9 100644 --- a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp +++ b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp @@ -29,10 +29,12 @@ void test_concurrent_creation() { vector iters; iters.reserve(1000); + + const auto iter = c.begin(); { - auto createIters = async(launch::async, [&]() { + auto copyIters = async(launch::async, [&]() { for (int i = 0; i < 1000; ++i) { - iters.push_back(c.begin()); + iters.push_back(iter); }; }); From b97c1bef5338450c31141aa4aa93914503dd7e08 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 15 Jul 2021 11:05:16 +0200 Subject: [PATCH 04/15] Only clear the container not destroy it --- tests/std/tests/GH_002058_debug_iterator_race/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp index 1cb137a0b9..41388ee5f9 100644 --- a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp +++ b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp @@ -38,7 +38,7 @@ void test_concurrent_creation() { }; }); - auto invalidateIters = async(launch::async, [&]() { c = container{}; }); + auto invalidateIters = async(launch::async, [&]() { c.clear(); }); } } From 845024643e4e021a1807642a74a69a29dd294668 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 15 Jul 2021 21:23:02 +0200 Subject: [PATCH 05/15] Address more review comments --- tests/std/tests/GH_002058_debug_iterator_race/test.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp index 41388ee5f9..2bed92c4ca 100644 --- a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp +++ b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp @@ -1,8 +1,6 @@ -#include #include #include #include -#include #include using namespace std; @@ -21,7 +19,7 @@ void test_concurrent_destruction() { } } -// Concurrently create iterators and invalidate container +// Concurrently create iterators and invalidate iterators template void test_concurrent_creation() { container c; From fee417f46a52097dc7791301bcd04e1a42b703b8 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 15 Jul 2021 21:25:05 +0200 Subject: [PATCH 06/15] Fix spelling --- stl/inc/xmemory | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index c4cf2122a6..29bc2350e4 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1228,7 +1228,7 @@ private: } _CONSTEXPR20_CONTAINER void _Orphan_me_unlocked() noexcept { - if (!_Myproxy) { // Already orphaned + if (!_Myproxy) { // already orphaned return; } From 6d2c96ddcd16ae80b78b56241da8ea72c6f73c07 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sun, 18 Jul 2021 07:59:44 +0200 Subject: [PATCH 07/15] Apply suggestions from code review Co-authored-by: Adam Bucior <35536269+AdamBucior@users.noreply.github.com> --- tests/std/tests/GH_002058_debug_iterator_race/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp index 2bed92c4ca..dc05a9dd4b 100644 --- a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp +++ b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp @@ -5,7 +5,7 @@ using namespace std; -// Concurrently destroy iterators and invalidate iterators +// Concurrently destroy and invalidate iterators template void test_concurrent_destruction() { container c; @@ -19,7 +19,7 @@ void test_concurrent_destruction() { } } -// Concurrently create iterators and invalidate iterators +// Concurrently create and invalidate iterators template void test_concurrent_creation() { container c; From bd4f30e120c5050610485192b9216bbcf7334dc3 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 Jul 2021 18:43:06 -0700 Subject: [PATCH 08/15] `#ifdef __cpp_lib_constexpr_dynamic_alloc` => `#if _HAS_CXX20` --- stl/inc/xmemory | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 2ce5a25e8f..ac39a41b42 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1112,11 +1112,11 @@ public: _CONSTEXPR20 _Iterator_base12& operator=(const _Iterator_base12& _Right) noexcept { #if _ITERATOR_DEBUG_LEVEL == 2 -#ifdef __cpp_lib_constexpr_dynamic_alloc +#if _HAS_CXX20 if (_STD is_constant_evaluated()) { _Assign_unlocked(_Right); } else -#endif // __cpp_lib_constexpr_dynamic_alloc +#endif // _HAS_CXX20 { _Assign_locked(_Right); } @@ -1134,33 +1134,33 @@ public: #if _ITERATOR_DEBUG_LEVEL == 2 _CONSTEXPR20 ~_Iterator_base12() noexcept { -#ifdef __cpp_lib_constexpr_dynamic_alloc +#if _HAS_CXX20 if (_STD is_constant_evaluated()) { _Orphan_me_unlocked(); } else -#endif // __cpp_lib_constexpr_dynamic_alloc +#endif // _HAS_CXX20 { _Orphan_me_locked(); } } _CONSTEXPR20 void _Adopt(const _Container_base12* _Parent) noexcept { -#ifdef __cpp_lib_constexpr_dynamic_alloc +#if _HAS_CXX20 if (_STD is_constant_evaluated()) { _Adopt_unlocked(_Parent); } else -#endif // __cpp_lib_constexpr_dynamic_alloc +#endif // _HAS_CXX20 { _Adopt_locked(_Parent); } } _CONSTEXPR20 void _Orphan_me_v2() noexcept { -#ifdef __cpp_lib_constexpr_dynamic_alloc +#if _HAS_CXX20 if (_STD is_constant_evaluated()) { _Orphan_me_unlocked(); } else -#endif // __cpp_lib_constexpr_dynamic_alloc +#endif // _HAS_CXX20 { _Orphan_me_locked(); } @@ -1265,11 +1265,11 @@ _CONSTEXPR20 void _Container_base12::_Orphan_all_unlocked() noexcept { _CONSTEXPR20 void _Container_base12::_Orphan_all() noexcept { #if _ITERATOR_DEBUG_LEVEL == 2 -#ifdef __cpp_lib_constexpr_dynamic_alloc +#if _HAS_CXX20 if (_STD is_constant_evaluated()) { _Orphan_all_unlocked(); } else -#endif // __cpp_lib_constexpr_dynamic_alloc +#endif // _HAS_CXX20 { _Orphan_all_locked(); } From 4c62e93d24355b2195b841891bf1028398928d06 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 Jul 2021 18:44:10 -0700 Subject: [PATCH 09/15] Use `native_matrix.lst` for ``. --- tests/std/tests/GH_002058_debug_iterator_race/env.lst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/GH_002058_debug_iterator_race/env.lst b/tests/std/tests/GH_002058_debug_iterator_race/env.lst index 19f025bd0e..e970fe46ae 100644 --- a/tests/std/tests/GH_002058_debug_iterator_race/env.lst +++ b/tests/std/tests/GH_002058_debug_iterator_race/env.lst @@ -1,4 +1,4 @@ # Copyright (c) Microsoft Corporation. # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -RUNALL_INCLUDE ..\usual_matrix.lst +RUNALL_INCLUDE ..\native_matrix.lst From 6ad37cdb918d991b4e05635ea0eea085bac7cf8e Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 Jul 2021 18:45:26 -0700 Subject: [PATCH 10/15] Add banner. --- tests/std/tests/GH_002058_debug_iterator_race/test.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp index dc05a9dd4b..358adce498 100644 --- a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp +++ b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + #include #include #include From 6e6fb72a95f912d537b9c303b965e28261f37b03 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 Jul 2021 18:45:46 -0700 Subject: [PATCH 11/15] Naming nitpick: `container` => `Container` --- .../std/tests/GH_002058_debug_iterator_race/test.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp index 358adce498..7dedf81b9b 100644 --- a/tests/std/tests/GH_002058_debug_iterator_race/test.cpp +++ b/tests/std/tests/GH_002058_debug_iterator_race/test.cpp @@ -9,12 +9,12 @@ using namespace std; // Concurrently destroy and invalidate iterators -template +template void test_concurrent_destruction() { - container c; + Container c; c.insert(c.begin(), 0); - vector iters(1000, c.begin()); + vector iters(1000, c.begin()); { auto destroyIters = async(launch::async, [&]() { iters.clear(); }); @@ -23,12 +23,12 @@ void test_concurrent_destruction() { } // Concurrently create and invalidate iterators -template +template void test_concurrent_creation() { - container c; + Container c; c.insert(c.begin(), 0); - vector iters; + vector iters; iters.reserve(1000); const auto iter = c.begin(); From a0869d13a89bc41ecb616c20564d4963d1d538b0 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 Jul 2021 19:31:18 -0700 Subject: [PATCH 12/15] Simplify IDL != 2 assignment; containers and proxies point to each other. --- stl/inc/xmemory | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index ac39a41b42..2ea3a48730 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1121,13 +1121,7 @@ public: _Assign_locked(_Right); } #else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 ^^^ / vvv _ITERATOR_DEBUG_LEVEL != 2 vvv - if (_Myproxy != _Right._Myproxy) { - if (_Right._Myproxy && _Right._Myproxy->_Mycont) { - _Myproxy = _Right._Myproxy->_Mycont->_Myproxy; - } else { - _Myproxy = nullptr; - } - } + _Myproxy = _Right._Myproxy; #endif // _ITERATOR_DEBUG_LEVEL != 2 return *this; } From eb2524979bfc5fae7199a9659b199c1fbe836c24 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 Jul 2021 19:40:36 -0700 Subject: [PATCH 13/15] Avoid unnecessary test of `_Right._Myproxy->_Mycont`. --- stl/inc/xmemory | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index 2ea3a48730..681f3db18a 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1186,7 +1186,7 @@ private: return; } - if (_Right._Myproxy && _Right._Myproxy->_Mycont) { + if (_Right._Myproxy) { _Adopt_unlocked(_Right._Myproxy->_Mycont); } else { // becoming invalid, disown current parent _Orphan_me_unlocked(); From 419920957441762b758bf6d7d3d1895f676a67e1 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 Jul 2021 20:04:38 -0700 Subject: [PATCH 14/15] Replace `_Orphan_me_v2()` with `_Adopt(nullptr)`. --- stl/inc/regex | 4 ++-- stl/inc/xmemory | 12 ------------ 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/stl/inc/regex b/stl/inc/regex index 02badc1ed9..d17ea56bb5 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 681f3db18a..e8d1b60de3 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1148,18 +1148,6 @@ public: _Adopt_locked(_Parent); } } - - _CONSTEXPR20 void _Orphan_me_v2() noexcept { -#if _HAS_CXX20 - if (_STD is_constant_evaluated()) { - _Orphan_me_unlocked(); - } else -#endif // _HAS_CXX20 - { - _Orphan_me_locked(); - } - } - #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 From 27fbe4ab7ed19d4716a2a23f39dad3c1761721f4 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 28 Jul 2021 21:33:11 -0700 Subject: [PATCH 15/15] Rename to `_v3` for ABI. --- stl/inc/xmemory | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/stl/inc/xmemory b/stl/inc/xmemory index e8d1b60de3..377249c774 100644 --- a/stl/inc/xmemory +++ b/stl/inc/xmemory @@ -1088,12 +1088,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 { @@ -1130,11 +1130,11 @@ public: _CONSTEXPR20 ~_Iterator_base12() noexcept { #if _HAS_CXX20 if (_STD is_constant_evaluated()) { - _Orphan_me_unlocked(); + _Orphan_me_unlocked_v3(); } else #endif // _HAS_CXX20 { - _Orphan_me_locked(); + _Orphan_me_locked_v3(); } } @@ -1177,7 +1177,7 @@ private: if (_Right._Myproxy) { _Adopt_unlocked(_Right._Myproxy->_Mycont); } else { // becoming invalid, disown current parent - _Orphan_me_unlocked(); + _Orphan_me_unlocked_v3(); } } @@ -1188,14 +1188,14 @@ private: _CONSTEXPR20 void _Adopt_unlocked(const _Container_base12* _Parent) noexcept { if (!_Parent) { - _Orphan_me_unlocked(); + _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(); + _Orphan_me_unlocked_v3(); } _Mynextiter = _Parent_proxy->_Myfirstiter; _Parent_proxy->_Myfirstiter = this; @@ -1208,7 +1208,7 @@ private: _Adopt_unlocked(_Parent); } - _CONSTEXPR20 void _Orphan_me_unlocked() noexcept { + _CONSTEXPR20 void _Orphan_me_unlocked_v3() noexcept { if (!_Myproxy) { // already orphaned return; } @@ -1225,15 +1225,15 @@ 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; } @@ -1249,11 +1249,11 @@ _CONSTEXPR20 void _Container_base12::_Orphan_all() noexcept { #if _ITERATOR_DEBUG_LEVEL == 2 #if _HAS_CXX20 if (_STD is_constant_evaluated()) { - _Orphan_all_unlocked(); + _Orphan_all_unlocked_v3(); } else #endif // _HAS_CXX20 { - _Orphan_all_locked(); + _Orphan_all_locked_v3(); } #endif // _ITERATOR_DEBUG_LEVEL == 2 }