Repository navigation
dgram: socket.send() crash when input array is modified #6616
Description
#4374 introduces a bug: it makes socket.send() accept an array but it's not resilient against that array getting modified afterwards:
$ node -e '
var a = ["boom!"], s = dgram.createSocket("udp4");
s.send(a, 1234);
a.splice(0);
'
node: ../deps/uv/src/unix/udp.c:390: uv__udp_send: Assertion `nbufs > 0' failed.
Aborted (core dumped)
A secondary issue with #4374 is that it penalizes the common case of passing in a buffer by always wrapping it in an array.
Activity
mcollina commentedon May 6, 2016
I tried hard to benchmark the array allocation penalty, and I failed to produce a statistical difference, at least when I did the PR.
Regarding the array change you are right, we need to clone. I hope it will not cause a perfomance penalty.
BTW, did this come from a user issue? Is there some urgency in fixing it? I will not have bandwidth till mid-may to work on it.
bnoordhuis commentedon May 6, 2016
I don't think it's critical. I was checking something in lib/dgram.js and this stood out.
mcollina commentedon May 6, 2016
@bnoordhuis let me know if you find any other area worth improving (there is actually http://docs.libuv.org/en/v1.x/udp.html#c.uv_udp_try_send to implement)
mcollina commentedon May 17, 2016
@brown-dragon there is a slight chance for a GC run to deallocate those buffers in the meanwhile, as the buffers are not copied. A new array needs to be allocated and all values copied over.
dgram: copy the list in send
dgram: copy the list in send
dgram: copy the list in send