Terminate immediately when allocation fails #3193
Conversation
str4d
added
C-upstream-port
src/prevector.h
Outdated
| _union.indirect = static_cast<char*>(realloc(_union.indirect, ((size_t)sizeof(T)) * new_capacity)); | ||
| assert(_union.indirect); |
There was a problem hiding this comment.
Would prefer:
if (!_union.indirect) { new_handler_terminate(); }
(and change the comment appropriately).
There was a problem hiding this comment.
... except that new_handler_terminate is static in init.cpp, but that can easily be fixed.
There was a problem hiding this comment.
I'm not particularly keen on this, because currently prevector.h has no dependency on any other local repository source file. Looking at upstream, the only local include they have introduced is on compat.h, which is a helper include that has no other local dependencies. If we wanted to re-use new_handler_terminate we'd at a minimum need to include util.h, which then means depending on libbitcoin_util.a anywhere prevector is used. That appears to be the case currently though...
There was a problem hiding this comment.
I don't see any problem with moving new_handler_terminate into util.cpp/h.
src/prevector.h
Outdated
| _union.capacity = new_capacity; | ||
| } else { | ||
| char* new_indirect = static_cast<char*>(malloc(((size_t)sizeof(T)) * new_capacity)); | ||
| assert(new_indirect); |
There was a problem hiding this comment.
if (!new_indirect) { new_handler_terminate(); }
src/init.cpp
Outdated
| // immediately to (try to) avoid chain corruption. | ||
| // Since LogPrintf may itself allocate memory, set the handler directly | ||
| // to terminate first. | ||
| std::set_new_handler(std::terminate); |
There was a problem hiding this comment.
Take @kallewoof's suggestion to do
fputs(stderr, "Error: Out of memory. Terminating.\n");
here (just before the LogPrintf).
str4d
force-pushed
the
b9856-badalloc-terminate
branch
from
00e07a2
to
aeb089e
Compare
There was a problem hiding this comment.
Tested ACK:
Sanity checked calling sequence via
char* new_indirect = NULL;
➜ src git:(b9856-badalloc-terminate) ✗ ./zcashd
Error: Out of memory. Terminating.
terminate called without an active exception
[1] 60500 abort (core dumped) ./zcashd
Otherwise performs as expected with suggested implementation
|
ACK. Ran same test as above. Results:
|
|
@zkbot r+ |
|
📌 Commit aeb089e has been approved by |
|
⌛ Testing commit aeb089e with merge 1fd3207b9e71e56de641cc4d48d8a794726162b4... |
|
💔 Test failed - pr-merge |
|
Unrelated transient RPC test failure.
@zkbot retry
…On Tue, 1 May 2018, 5:55 AM Homu, ***@***.***> wrote:
💔 Test failed - pr-merge <https://ci.z.cash/#builders/3/builds/279>
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
<#3193 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEwzBzxmUMLBspfsKCldqGML-BJq8Xr6ks5tt-q-gaJpZM4TcDpM>
.
|
Terminate immediately when allocation fails Backport of bitcoin/bitcoin#9856 Closes #1498.
Backport of bitcoin/bitcoin#9856
Closes #1498.