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

_ITERATOR_DEBUG_LEVEL 2: crash when iterators are destroyed while container is modified #2058

Closed
decden opened this issue Jul 14, 2021 · 2 comments · Fixed by #2060
Closed
Labels
bug Something isn't working fixed Something works now, yay!

Comments

@decden
Copy link

decden commented Jul 14, 2021

Iterator debug level 2 does not support concurrent deletion and invalidation of iterators from two different threads. When this happens, the application crashes with a nullptr access inside of the STL.

Command-line test case

C:\Temp>type repro.cpp
#include <iostream>
#include <unordered_set>
#include <future>
#include <thread>
#include <chrono>
#include <vector>

int main() {
	// Create a one element set
	std::unordered_set<int> set;
	set.insert(0);

	// Create a list of iterators to later destroy
	std::vector<std::unordered_set<int>::iterator> iters;
	for (int i = 0; i < 1000; ++i)
		iters.push_back(set.begin());

	// Concurrently destroy iterators and invalidate iterators
	{
		auto destroyIters = std::async(std::launch::async,
			[&](){
				iters.clear();
			});

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

	std::cout << "Programm behaved as expected, and did not crash" << std::endl;
}

C:\Temp>cl /EHsc /W4 /WX /Zi /DEBUG /MTd /D_ITERATOR_DEBUG_LEVEL=2 .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30038.1 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

repro.cpp
Microsoft (R) Incremental Linker Version 14.29.30038.1
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:repro.exe
/debug
repro.obj

C:\Temp>.\repro.exe
[application crashes]

Expected behavior
The application should not crash.

STL version
Microsoft Visual Studio Professional 2019 Version 16.10.2

@decden decden changed the title _ITERATOR_DEBUG_LEVEL 2: Apps crash when iterators are destroyed while container is modified _ITERATOR_DEBUG_LEVEL 2: crash when iterators are destroyed while container is modified Jul 14, 2021
@miscco
Copy link
Contributor

miscco commented Jul 14, 2021

So this is a race condition in the debug iterator machinery.

Note that due to constexpr containers and the restrictions during constant evaluation we need to have special paths for constant evaluation and plain runtime.

Now the bug is here:

    _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
#endif // __cpp_lib_constexpr_dynamic_alloc
            {
                _Orphan_me_locked();
            }
        }
    }

We check _Myproxy before we take the lock, which introduces a race condition.

the proper solution is to only perform the if (_Myproxy) inside the _Orphan_me_unlocked call

miscco added a commit to miscco/STL that referenced this issue Jul 14, 2021
@CaseyCarter CaseyCarter added the bug Something isn't working label Jul 14, 2021
@CaseyCarter CaseyCarter changed the title _ITERATOR_DEBUG_LEVEL 2: crash when iterators are destroyed while container is modified _ITERATOR_DEBUG_LEVEL 2: crash when iterators are destroyed while container is modified Jul 14, 2021
@StephanTLavavej StephanTLavavej added fixed Something works now, yay! and removed work in progress labels Jul 30, 2021
@StephanTLavavej
Copy link
Member

Totally unrelated, but passing /DEBUG to the compiler will define a macro named EBUG. Passing /MTd is what activates debug mode (and defines the macro _DEBUG).

(There's a linker option /DEBUG, which is distinct from "debug mode".)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Something works now, yay!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants