Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when writing huge of content to a file json #13465

Closed
trungducng opened this issue Jun 5, 2017 · 28 comments
Closed

Error when writing huge of content to a file json #13465

trungducng opened this issue Jun 5, 2017 · 28 comments
Labels
buffer Issues and PRs related to the buffer subsystem.

Comments

@trungducng
Copy link

<--- JS stacktrace —->

==== JS stack trace =========================================

Security context: 0x3b902a9266a1 <JS Object>
    1: fromString(aka fromString) [buffer.js:~194] [pc=0x1b9dcb6f3ad0](this=0x2f5cceb02311 <undefined>,string=0x866865dc389 <Very long 
string[14371]>,encoding=0x3b902a92f309 <String[4]: utf8>)
    2: from [buffer.js:~96] [pc=0x1b9dcb6c9936](this=0x2bea42122009 <JS Function Buffer (SharedFunctionInfo 0x3b902a94b481)>,value=0x86
6865dc389 <Very long string[14371]>,encodingOrOffset=0x3b902a92f309 <...

FATAL ERROR: CALL_AND_RETRY_LAST All

Error when writing huge of content to a file json, Can anyone help me solve this propblem? I tried

node --max-old-space-size=8192 index.js

But it still help me about out of memory in heap problem

@vsemozhetbyt
Copy link
Contributor

Could it be related to String size limit? See #9489

What is String length you try to output in JSON?

@trungducng
Copy link
Author

@vsemozhetbyt The JSON file is more than 500mb when I carry out writing

@vsemozhetbyt
Copy link
Contributor

Could you provide a small code fragment to reproduce the issue?

@vsemozhetbyt
Copy link
Contributor

cc @nodejs/buffer ?

@vsemozhetbyt vsemozhetbyt added the buffer Issues and PRs related to the buffer subsystem. label Jun 5, 2017
@addaleax
Copy link
Member

addaleax commented Jun 5, 2017

Yes, you are either hitting the string length limit (which I thought was 256 MB?), or are going out of memory, neither of which are very unlikely when handling strings this huge.

I agree with @vsemozhetbyt, in order to see if we are even able to help you, you’d need to provide enough code to understand exactly what is going on. You might also want to look into the possibility of using a streaming JSON module to handle your data.

@vsemozhetbyt
Copy link
Contributor

@addaleax

de facto limit for the current v8: 268,435,440 characters (Math.pow(2, 28) - 16), 536,870,880 bytes in UTF16.

(#9489)

These issues repeat themselves. Maybe we should add something about String limit to docs (buffer.md?), with some workarounds like the mentioned streaming module.

@addaleax
Copy link
Member

addaleax commented Jun 5, 2017

Maybe we should add something about String limit to docs (buffer.md?), with some workarounds like the mentioned streaming module.

Sounds reasonable, yes. Maybe we should also export V8’s String::kMaxLength to JS, like buffer.kMaxLength, what do you think?

@vsemozhetbyt
Copy link
Contributor

Will this allow to increase the String limit for v8? It is hard for me to assess the effects.

cc @nodejs/v8 ?

@addaleax
Copy link
Member

addaleax commented Jun 5, 2017

@vsemozhetbyt I’m talking about just providing the value to JS scripts, not to make it mutable (I have no idea if that would work). That might be useful for giving programmers a way to tell whether something can even be turned into a string, or whether it’s too large.

@tniessen
Copy link
Member

tniessen commented Jun 5, 2017

Why is this limit in place? Why doesn't v8 allow arbitrary strings as long as it does not run out of heap space?

@addaleax
Copy link
Member

addaleax commented Jun 5, 2017

That’s really a question for @nodejs/v8. ;)

@vsemozhetbyt
Copy link
Contributor

Maybe we should also foresee the cases when using this check is not so obvious. For example:

fs.writeFileSync('file.json', JSON.stringify(hugeObject), 'utf16le');

Maybe some checks should be carried out by buffer or fs themselves, with a clear error message.

@addaleax
Copy link
Member

addaleax commented Jun 5, 2017

@vsemozhetbyt I don’t quite get your example here. By the time JSON.stringify is finished, the string has already been created, and writing the data should work fine as far as Node is concerned. If you are talking about JSON.stringify throwing a certain (better?) error… I don’t know, but that’s not under Node’s control, right?

@vsemozhetbyt
Copy link
Contributor

@addaleax I just mean that sometimes a user cannot even see an intermediate String issue to think of a limit check. Yes, maybe this is not the best example. Then how a user can use a check? Should the buffer size be checked? How a user can check if hugeObject will not exceed the String limit if the JSON.stringify fails?

@addaleax
Copy link
Member

addaleax commented Jun 5, 2017

How a user can check if hugeObject will not exceed the String limit if the JSON.stringify fails?

I don’t think that’s possible. :/ In that case, a streaming JSON generator might be the only solution that allows you to be somewhat sure that everything works.

@vsemozhetbyt
Copy link
Contributor

Then the only use of proposed require('buffer').kStringMaxLength is a check of a buffer size before the string conversion, right?

@addaleax
Copy link
Member

addaleax commented Jun 5, 2017

@vsemozhetbyt There are a lot of other ways to generate long strings but that’s what I’m having in mind, yes

@tniessen
Copy link
Member

tniessen commented Jun 5, 2017

This does not really affect the problem @trungducng has, right? If the result of JSON.stringify() was actually longer than kStringMaxLength, we should see a RangeError: Invalid string length. Or am I missing something?

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Jun 5, 2017

FWIW:

console.log('*'.repeat(Math.pow(2, 28) - 16).length);
console.log(JSON.stringify('*'.repeat(Math.pow(2, 28) - 16 - 1)).length);
268435440
test.js:5
console.log(JSON.stringify('*'.repeat(Math.pow(2, 28) - 16 - 1)).length);
                 ^

RangeError: Invalid string length
    at JSON.stringify (<anonymous>)
    at Object.<anonymous> (test.js:5:18)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Function.Module.runMain (module.js:605:10)
    at startup (bootstrap_node.js:158:16)
    at bootstrap_node.js:575:3

Maybe some concatenations of JSON strings have a place.

@addaleax
Copy link
Member

addaleax commented Jun 5, 2017

@vsemozhetbyt I think that would work if you use - 2 instead of - 1 to account for the quotes at the beginning/end of the JSON string :)

@tniessen
Copy link
Member

tniessen commented Jun 5, 2017

What seems to fail is converting the string returned by JSON.stringify() to a buffer. Note that at this point, the object, its JSON representation and a buffer of similar (or half the) size are in memory, and that's a lot. You can try to work around this issue by writing the content in chunks so you won't need to keep the whole buffer in memory. Or use a streaming JSON generator as suggested.

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Jun 5, 2017

Yes, I've just confimed what @tniessen said) This is OK:

console.log('*'.repeat(Math.pow(2, 28) - 16).length);
console.log(JSON.stringify('*'.repeat(Math.pow(2, 28) - 16 - 2)).length);

@bnoordhuis
Copy link
Member

Why is this limit in place? Why doesn't v8 allow arbitrary strings as long as it does not run out of heap space?

There is talk of raising it: https://bugs.chromium.org/p/v8/issues/detail?id=6148

@mscdex
Copy link
Contributor

mscdex commented Jun 5, 2017

Other VMs might use different max string length values. Is this something that could be communicated via n-api perhaps?

@sam-github
Copy link
Contributor

Then the only use of proposed require('buffer').kStringMaxLength is a check of a buffer size before the string conversion, right?

I think it has at least one other useful side-effect, it would show up in the TOC, and the docs for it would make clear that there is a maximum. Without it, I'm not sure where in the docs the max string size docs would go.

addaleax added a commit to addaleax/node that referenced this issue Jun 7, 2017
Add `buffer.constants`, containing length limits for `Buffer` and
`string` instances.

This could be useful for programmers to tell whether a value can
be turned into a string or not.

Ref: nodejs#13465
addaleax added a commit that referenced this issue Jun 16, 2017
Add `buffer.constants`, containing length limits for `Buffer` and
`string` instances.

This could be useful for programmers to tell whether a value can
be turned into a string or not.

Ref: #13465
PR-URL: #13467
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax added a commit that referenced this issue Jun 17, 2017
Add `buffer.constants`, containing length limits for `Buffer` and
`string` instances.

This could be useful for programmers to tell whether a value can
be turned into a string or not.

Ref: #13465
PR-URL: #13467
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax added a commit that referenced this issue Jun 21, 2017
Add `buffer.constants`, containing length limits for `Buffer` and
`string` instances.

This could be useful for programmers to tell whether a value can
be turned into a string or not.

Ref: #13465
PR-URL: #13467
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax added a commit that referenced this issue Jun 24, 2017
Add `buffer.constants`, containing length limits for `Buffer` and
`string` instances.

This could be useful for programmers to tell whether a value can
be turned into a string or not.

Ref: #13465
PR-URL: #13467
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
rvagg pushed a commit that referenced this issue Jun 29, 2017
Add `buffer.constants`, containing length limits for `Buffer` and
`string` instances.

This could be useful for programmers to tell whether a value can
be turned into a string or not.

Ref: #13465
PR-URL: #13467
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
addaleax added a commit that referenced this issue Jul 11, 2017
Add `buffer.constants`, containing length limits for `Buffer` and
`string` instances.

This could be useful for programmers to tell whether a value can
be turned into a string or not.

Ref: #13465
PR-URL: #13467
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@targos
Copy link
Member

targos commented Sep 10, 2017

Can we consider this (and #9489) to be fixed by #13467?

@dandv
Copy link
Contributor

dandv commented Oct 23, 2019

Yes, you are either hitting the string length limit (which I thought was 256 MB?), or are going out of memory, neither of which are very unlikely when handling strings this huge.

I've been seeing this OOM crash repeatedly with far shorter JSON. My code writes a report of constant size periodically to disk, as data is being updated:

fs.writeFile(reportFilename, JSON.stringify(report, null, 2), err => { if (err) throw err; });

When it succeeds (most of the time), the size of the file (including 2-space indentation) is ~18MB, and he file is ~620,000 lines long. (I'd be happy to share if it weren't proprietary data.) However, one in about 150-250 tries, Node crashes with an Out of Memory error. I've attached a sample report.

OOM-report.zip

I'm running node v12.11.1 with --max_old_space_size=2048.

@hello-smile6
Copy link

This same bug kills Chromium 61 Android with a giant array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem.
Projects
None yet
Development

No branches or pull requests

10 participants