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

deque(size_t,const Allocator&) doesn't clear up constructed elements on exception #3717

Closed
achabense opened this issue May 19, 2023 · 1 comment · Fixed by #3720
Closed
Labels
bug Something isn't working fixed Something works now, yay!

Comments

@achabense
Copy link
Contributor

achabense commented May 19, 2023

deque<T>(size_t,const Allocator&) lacks a tidy guard now. When the default constructor of a T throws, no destructor of constructed Ts will be called.

For example, in the code below, test([] {std::deque<resource>f(10); }); will print "created:5 destroyed:0". The expected behavior should be "created:5 destroyed:5" as the other code do.

#include<deque>
#include<iostream>

struct resource {
	inline static int created = 0, destroyed = 0;
	resource() {
		if (created == 5) { throw 0; }
		++created;
	}
	resource(const resource&) {
		if (created == 5) { throw 0; }
		++created;
	}
	~resource() { ++destroyed; }
};

void test(auto f) {
	resource::created = resource::destroyed = 0;
	try { f(); }
	catch (...) {}
	std::cout << "created:" << resource::created << " destroyed:" << resource::destroyed << std::endl;
}

int main() {
	//created:5 destroyed:0
	test([] {std::deque<resource>f(10); });
	//created:5 destroyed:5
	test([] {std::deque<resource>f; f.resize(10); });
	test([] {std::deque<resource>f; f.resize(10, resource{}); });
	test([] {std::deque<resource>f(10, resource{}); });
	test([] {std::deque<resource>f(10, resource{}, {}); });
}

Causes: In the implementation, the use of resize is wrong as it doesn't do cleanup on exception, and the construction is not completed so ~deque() won't be called either.

STL/stl/inc/deque

Lines 622 to 628 in a621095

explicit deque(_CRT_GUARDOVERFLOW size_type _Count, const _Alloc& _Al = _Alloc())
: _Mypair(_One_then_variadic_args_t{}, _Al) {
_Alproxy_ty _Alproxy(_Getal());
_Container_proxy_ptr12<_Alproxy_ty> _Proxy(_Alproxy, _Get_data());
resize(_Count);
_Proxy._Release();
}

@achabense achabense changed the title <deque>(size_t,const Allocator&) doesn't handle exceptions properly <deque>(size_t,const Allocator&) doesn't clear up constructed elements on exception May 19, 2023
@achabense achabense changed the title <deque>(size_t,const Allocator&) doesn't clear up constructed elements on exception deque(size_t,const Allocator&) doesn't clear up constructed elements on exception May 19, 2023
CaseyCarter added a commit to CaseyCarter/STL that referenced this issue May 19, 2023
@CaseyCarter CaseyCarter added the bug Something isn't working label May 19, 2023
@CaseyCarter
Copy link
Contributor

CaseyCarter commented May 19, 2023

That's a nasty bug; thanks for the report.

@StephanTLavavej StephanTLavavej added the fixed Something works now, yay! label May 30, 2023
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.

3 participants