Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race condition in iterator debug machinery #2060

Merged
merged 17 commits into from
Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 34 additions & 34 deletions stl/inc/xmemory
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

Expand All @@ -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) {
miscco marked this conversation as resolved.
Show resolved Hide resolved
_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
miscco marked this conversation as resolved.
Show resolved Hide resolved
_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 {
Expand Down
1 change: 1 addition & 0 deletions tests/std/test.lst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions tests/std/tests/GH_002058_debug_iterator_race/env.lst
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

RUNALL_INCLUDE ..\usual_matrix.lst
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
51 changes: 51 additions & 0 deletions tests/std/tests/GH_002058_debug_iterator_race/test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#include <chrono>
miscco marked this conversation as resolved.
Show resolved Hide resolved
#include <future>
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
#include <list>
#include <set>
#include <thread>
miscco marked this conversation as resolved.
Show resolved Hide resolved
#include <vector>

using namespace std;

// Concurrently destroy iterators and invalidate iterators
miscco marked this conversation as resolved.
Show resolved Hide resolved
template <class container>
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
void test_concurrent_destruction() {
container c;
c.insert(c.begin(), 0);

vector<typename container::iterator> iters(1000, c.begin());
{
auto destroyIters = async(launch::async, [&]() { iters.clear(); });

auto invalidateIters = async(launch::async, [&]() { c.clear(); });
}
}

// Concurrently create iterators and invalidate container
miscco marked this conversation as resolved.
Show resolved Hide resolved
template <class container>
void test_concurrent_creation() {
container c;
c.insert(c.begin(), 0);

vector<typename container::iterator> 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{}; });
CaseyCarter marked this conversation as resolved.
Show resolved Hide resolved
}
}

int main() {
test_concurrent_destruction<list<int>>();
test_concurrent_destruction<set<int>>();
test_concurrent_destruction<vector<int>>();

test_concurrent_creation<list<int>>();
test_concurrent_creation<set<int>>();
test_concurrent_creation<vector<int>>();
}