-
Notifications
You must be signed in to change notification settings - Fork 7.3k
v0.12.0: upstream fix to --max_old_space_size=2048 integer overflow #9200
Conversation
Thanks for reporting. Unfortunately I can't reproduce the issue on my x64 Linux box. What are you running? |
repro.js: var a = { };
for (var i = 0; i < 1000000; i++)
a[i] = i;
|
Hm. I get a different error message:
|
Original commit message: Fix --max_old_space_size=4096 integer overflow. BUG=v8:3857 LOG=N Review URL: https://codereview.chromium.org/897543002 Cr-Commit-Position: refs/heads/master@{nodejs#26510}
@trevnorris, did you try it with v0.12.0? It looks like a different issue preventing the reported one from happening. |
We're also OOM errors on 0.12 64bit Linux. Running node with --max_old_space_size=6144.
|
I guess #9185 should be merged first. |
@bsnote Sorry. I can't reproduce the issue you're having. Here's the script:
Please share your system specs and anything else you think might be helpful for me to reproduce this issue. |
@trevnorris I can repro on my AWS Instance. We're running r3.large instance types.
|
@yunong are you using the exec from the website, or building your own? |
@yunong Okay, just took the download from the website and was able to reproduce. @tjfontaine @misterdjules There's something different between the builds that I'm using and the ones from the website that's causing this issue. |
@trevnorris I'm using the 64bit Linux binary downloaded from nodejs.org |
The amount of memory used isn't close to even the default maximum allowed:
Only 61MB? |
@trevnorris The Linux binaries available on the website are built with |
@trevnorris at the very least it will be your compiler version, the older compiler here is probably tickling this overflow resulting in the failure |
@tjfontaine That must be it. I ran
I also just confirmed this is happening with io.js. Using https://iojs.org/dist/v1.2.0/iojs-v1.2.0-linux-x64.tar.gz I was able to reproduce the issue:
This may be a V8 bug. /cc @bnoordhuis @indutny |
Anything wrong with the V8 upstream fix by @bnoordhuis that I attached to this issue? |
@bsnote I totally missed the mention of the V8 issue and fix in your original comment, sorry for the confusion. I think @trevnorris might have missed that too, hence his question. |
Ah yes. I did miss the linked V8 issue. |
LGTM. |
@trevnorris LGTM, thanks! |
Original commit message: Fix --max_old_space_size=4096 integer overflow. BUG=v8:3857 LOG=N Review URL: https://codereview.chromium.org/897543002 Cr-Commit-Position: refs/heads/master@{#26510} PR-URL: #9200 Reviewed-by: Trevor Norris <trev.norris@gmail.com> Reviewed-by: Julien Gilli <julien.gilli@joyent.com>
@trevnorris Sorry if I'm missing this but will the fix be in an actual node.js release? 0.12.1 ? |
@edgework Yes, this fix will be in node v0.12.1. |
v0.12.0 crashes when I specify --max_old_space_size=2048 or greater:
FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - process out of memory
https://code.google.com/p/v8/issues/detail?id=3857
https://codereview.chromium.org/897543002