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

[c++] Recovery from out-of-heap-memory condition is not possible #368

Closed
chris-durand opened this issue Mar 22, 2020 · 4 comments · Fixed by #364
Closed

[c++] Recovery from out-of-heap-memory condition is not possible #368

chris-durand opened this issue Mar 22, 2020 · 4 comments · Fixed by #364

Comments

@chris-durand
Copy link
Member

chris-durand commented Mar 22, 2020

While reading through the changes of #364 I noticed modm's operator new doesn't allow to recover from out-of-memory conditions. In the C++ standard library there is the so-called "new handler". It is a function that should be called whenever operator new fails to allocate memory. Cppreference on operator new:

In case of failure, the standard library implementation calls the function pointer returned by std::get_new_handler and repeats allocation attempts until new handler does not return or becomes a null pointer, at which time it throws std::bad_alloc. This function is required to return a pointer suitably aligned to point to an object of the requested size.

In a conforming standard library implementation the user can override the new handler with std::set_new_handler() to provide a function that frees some memory in an out of memory condition. The modm version does not use the new handler.

In #343 modm added the option to use exceptions. Shouldn't the throwing operator new actually throw on out-of-memory when exceptions are enabled?

I also noticed operator new asserts with modm_assert_continue_fail() for AVR, but has a normal modm_assert() on Cortex-M. I'm not sure continuing is the right way to attempt a recovery. For example, getting back a null pointer from new inside the implementation of std::vector will certainly crash your program.

@salkinium
Copy link
Member

salkinium commented Mar 22, 2020

Ok, I'll incorporate this into #364. I'm glad you're paying attention, cos C++ conformance isn't so strong in this one 😳.

(Why is this an issue, not a review? ;-)

@salkinium
Copy link
Member

salkinium commented Mar 22, 2020

Shouldn't the throwing operator new actually throw on out-of-memory when exceptions are enabled?

You mean instead of calling modm_assert? Otherwise you'd have to throw after modm_assert, and then it's kinda duplicate? For maximum confusion you could add an assertion handler that checks if new failed and then throws std::bad_alloc. BWAHARHAR!

@salkinium
Copy link
Member

repeats allocation attempts until new handler does not return [...] at which time it throws std::bad_alloc.

Glad to see C++ solved the halting problem… lol.

@salkinium
Copy link
Member

I've implemented all of this in #364.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants