Prevent undefined behavior when some Node Buffer objects are destroyed #7452

Merged
merged 1 commit into from Oct 3, 2016

Projects

None yet

2 participants

@enlight
Contributor
enlight commented Oct 2, 2016 edited

If node::Buffer::New() is used to wrap an existing chunk of memory without providing a custom callback to release that memory then Node will just use free(). In a couple of places Node buffer objects were constructed from chunks of memory that were allocated with new[], but a custom callback to release that memory was omitted, this resulted in undefined behavior when those buffers were destroyed because free() was used to release memory allocated with new[].

To avoid undefined behavior the aforementioned buffer objects are now constructed with a custom callback that safely releases the underlying chunk of memory.

@enlight
Contributor
enlight commented Oct 2, 2016

Oops, maybe using malloc isn't such a great idea since it means you have to check if it returned nullptr. Will revise this in the morning.

@enlight enlight Prevent undefined behavior when some Node Buffer objects are destroyed
If node::Buffer::New() is used to wrap an existing chunk of memory
without providing a custom callback to release that memory then Node
will just use `free()`. In a couple of places Node buffer objects were
constructed from chunks of memory that were allocated with `new[]`, but
a custom callback to release that memory was omitted, this resulted in
undefined behavior when those buffers were destroyed because `free()`
was used to release memory allocated with `new[]`.

To avoid undefined behavior the aforementioned buffer objects are now
constructed with a custom callback that safely releases the underlying
chunk of memory.
7c5d329
@enlight
Contributor
enlight commented Oct 3, 2016

OK back to using new[], which means Electron will just crash if there isn't sufficient memory to allocate any of the buffers.

@zcbenz
Contributor
zcbenz commented Oct 3, 2016

👍

@zcbenz zcbenz merged commit 9833304 into electron:master Oct 3, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment