dns: fix empty elem in result array#3696
Uh oh!
There was an error while loading. Please reload this page.
Conversation
When getaddrinfo linked-list results contain entries other than AF_INET and AF_INET6, the resulting v8::Array will contain undefined value. That's because initialization of v8::Array pre-allocate entries for all linked-list nodes, but we may actually some of them after the 2 while loops.
john-yan commented Nov 6, 2015
This fixes intermittent failure of test/internet/test-dns-ipv6.js caused by assertion failure on line 168. ips.forEach(function(ip){assert.ok(isIPv6(ip.address),// failing assertion'Invalid IPv6: '+ip.address.toString());assert.strictEqual(ip.family,6);});I observed that the ip.address is undefined. then I try to print out the whole ips, and found this output: then I try to print out the parameter "addresses" in onlookupall in lib/dns.js: |
cjihrig commented Nov 6, 2015
LGTM, but I'd like the input of someone more familiar with that part of the code base. Are we going to take a hit somewhere by not passing |
john-yan commented Nov 6, 2015
@cjihrig btw, the default length passing to Array::New is 0. |
mhdawson commented Nov 6, 2015
If there is some concern with not passing a larger default length, can we remove the entries that have an undefined address after the fact ? |
cjihrig commented Nov 6, 2015
Seems like a question @trevnorris might know the answer to. Is it likely faster to create a larger array up front and then remove undefined values, or build the array as needed? It may also depend on how common |
john-yan commented Nov 6, 2015
@cjihrig undefined value is a special double representation (8 bytes). |
john-yan commented Nov 6, 2015
@mhdawson allocating a larger array and then removing entries would increase internal fragmentation. It may not benefit performance. |
trevnorris commented Nov 6, 2015
Without testing, I would say each are equally fast or slow. Wouldn't worry about that amount of possible performance gain here. |
cjihrig commented Nov 6, 2015
In that case, I'd say this is fine as long as the CI is happy. |
john-yan commented Nov 6, 2015
@trevnorris absolutely agree. |
bnoordhuis commented Nov 6, 2015
LGTM, this is not performance critical. At least, not so critical that specifying the length upfront makes a difference. |
john-yan commented Nov 10, 2015
Hello, can someone merge this in? It has been 4 days now. :) |
jasnell commented Nov 10, 2015
@john-yan .. there were some failures in the CI on freebsd. They look unrelated but can you please take a look and confirm. |
john-yan commented Nov 10, 2015
@jasnell Hello, the failures show some native modules not found. They should be unrelated. |
cjihrig commented Nov 10, 2015
Yes, this looks like it should be fine. Landing. |
When getaddrinfo linked-list results contain entries other than AF_INET and AF_INET6, the resulting v8::Array will contain undefined values. That's because initialization of v8::Array pre-allocates entries for all linked-list nodes, but not all of them will be in the final results. This commit ensures that the array only contains valid results. PR-URL: #3696 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
cjihrig commented Nov 10, 2015
Thanks! Landed in 19bf72d |
When getaddrinfo linked-list results contain entries other than AF_INET and AF_INET6, the resulting v8::Array will contain undefined values. That's because initialization of v8::Array pre-allocates entries for all linked-list nodes, but not all of them will be in the final results. This commit ensures that the array only contains valid results. PR-URL: #3696 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
When getaddrinfo linked-list results contain entries other than AF_INET and AF_INET6, the resulting v8::Array will contain undefined values. That's because initialization of v8::Array pre-allocates entries for all linked-list nodes, but not all of them will be in the final results. This commit ensures that the array only contains valid results. PR-URL: nodejs/node#3696 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
When getaddrinfo linked-list results contain entries other than AF_INET and AF_INET6, the resulting v8::Array will contain undefined values. That's because initialization of v8::Array pre-allocates entries for all linked-list nodes, but not all of them will be in the final results. This commit ensures that the array only contains valid results. PR-URL: #3696 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins commented Nov 17, 2015
landed in v4.x-staging in a632db5 |
When getaddrinfo linked-list results contain entries other than AF_INET and AF_INET6, the resulting v8::Array will contain undefined values. That's because initialization of v8::Array pre-allocates entries for all linked-list nodes, but not all of them will be in the final results. This commit ensures that the array only contains valid results. PR-URL: #3696 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
When getaddrinfo linked-list results contain entries other than AF_INET and AF_INET6, the resulting v8::Array will contain undefined values. That's because initialization of v8::Array pre-allocates entries for all linked-list nodes, but not all of them will be in the final results. This commit ensures that the array only contains valid results. PR-URL: #3696 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
When getaddrinfo linked-list results contain entries other than AF_INET and AF_INET6, the resulting v8::Array will contain undefined values. That's because initialization of v8::Array pre-allocates entries for all linked-list nodes, but not all of them will be in the final results. This commit ensures that the array only contains valid results. PR-URL: #3696 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
8 participants
When getaddrinfo linked-list results contain entries other than AF_INET
and AF_INET6, the resulting v8::Array will contain undefined value.
That's because initialization of v8::Array pre-allocate entries for all
linked-list nodes, but we may actually some of them after the 2 while
loops.