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

[editor] Opening a big binary file crashes the whole backend #3859

Closed
paul-marechal opened this issue Dec 18, 2018 · 9 comments · Fixed by #8152
Closed

[editor] Opening a big binary file crashes the whole backend #3859

paul-marechal opened this issue Dec 18, 2018 · 9 comments · Fixed by #8152
Assignees
Labels
bug bugs found in the application editor issues related to the editor filesystem issues related to the filesystem

Comments

@paul-marechal
Copy link
Member

Crash log:

<--- Last few GCs --->

[11820:0x415d700]   166071 ms: Mark-sweep 1031.0 (1116.6) -> 1030.7 (1121.1) MB, 36.6 / 0.0 ms  allocation failure GC in old space requested
[11820:0x415d700]   166111 ms: Mark-sweep 1030.7 (1121.1) -> 1030.6 (1086.6) MB, 39.8 / 0.0 ms  last resort GC in old space requested
[11820:0x415d700]   166148 ms: Mark-sweep 1030.6 (1086.6) -> 1030.6 (1086.6) MB, 37.0 / 0.0 ms  last resort GC in old space requested


<--- JS stacktrace --->

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

Security context: 0x23b3922a5879 <JSObject>
    0: builtin exit frame: stringify(this=0x23b3922890a9 <Object map = 0x39e1fce02ba1>,0x2290dab822d1 <undefined>,0x2290dab822d1 <undefined>,0x21b88d57a0a1 <Object map = 0x12c37fdcbcb9>)

    1: arguments adaptor frame: 1->3
    2: send [/home/huehuehue/projects/theia/packages/core/lib/common/messaging/web-socket-channel.js:83] [bytecode=0x10c769d55ac1 offset=47](this=0x158c3e947501 <WebSocketChan...

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
 1: node::Abort() [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
 2: 0x8c21ec [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
 3: v8::Utils::ReportOOMFailure(char const*, bool) [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
 4: v8::internal::V8::FatalProcessOutOfMemory(char const*, bool) [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
 5: v8::internal::Factory::NewRawTwoByteString(int, v8::internal::PretenureFlag) [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
 6: v8::internal::String::SlowFlatten(v8::internal::Handle<v8::internal::ConsString>, v8::internal::PretenureFlag) [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
 7: v8::internal::JsonStringifier::SerializeString(v8::internal::Handle<v8::internal::String>) [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
 8: v8::internal::JsonStringifier::Result v8::internal::JsonStringifier::Serialize_<true>(v8::internal::Handle<v8::internal::Object>, bool, v8::internal::Handle<v8::internal::Object>) [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
 9: v8::internal::JsonStringifier::Result v8::internal::JsonStringifier::Serialize_<false>(v8::internal::Handle<v8::internal::Object>, bool, v8::internal::Handle<v8::internal::Object>) [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
10: v8::internal::JsonStringifier::Stringify(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>) [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
11: v8::internal::Builtin_JsonStringify(int, v8::internal::Object**, v8::internal::Isolate*) [/home/huehuehue/.nvm/versions/node/v8.11.3/bin/node]
12: 0x38f010a8697d
Server worker has been disconnected. [ID: 1 | PID: 11820]
Server worker has been stopped. [ID: 1 | PID: 11820]

On the other hand VS Code does manage to open such big files (123.73MB), even though at first it will tell you that it doesn't know how to properly render it:

image

But once you do open it, VS Code seems to be optimizing things so that it doesn't die like Theia do:

image

@paul-marechal paul-marechal added bug bugs found in the application filesystem issues related to the filesystem editor issues related to the editor labels Dec 18, 2018
@paul-marechal paul-marechal changed the title Opening a big binary file crashes the whole backend [editor] Opening a big binary file crashes the whole backend Dec 18, 2018
@a1994846931931
Copy link
Contributor

And a zip file as well. I think there are at least two problems relating to this issue:

  1. 'Preview' should not display some type of files. Consider how vscode does this: if you click your mouse on an "exe" file in vscode, the preview window shows a message "The file is not displayed in the editor because it is either binary or uses an unsupported text encoding. Do you want to open it anyway?"
  2. Transferring big files should not block the whole backend.

Also, consider:

  • Show warning when trying to open big files

@akosyakov
Copy link
Member

akosyakov commented May 28, 2019

Transferring big files should not block the whole backend.

that's a bit tricky, we either have to redesign FileSystem API in order to allow streaming files, i.e. content is loaded in small chunks. It will allow to reuse the same web socket. Or we should fetch content with a dedicated HTTP request instead through the web socket. The same applies to saving content.

@paul-marechal
Copy link
Member Author

paul-marechal commented May 28, 2019

It might be easier to use vanilla HTTP, since there are APIs to do that kind of streaming.

Maybe we could even load every file like this. Although I wouldn't know how to plug the monaco editor to such a stream. But it will be nice to load the chunks progressively in the editors too and not just buffer while transmitting.

@akosyakov
Copy link
Member

Maybe we could even load every file like this. Although I wouldn't know how to plug the monaco editor to such a stream. But it will be nice to load the chunks progressively in the editors too and not just buffer while transmitting.

As far as I know monaco has to keep the whole document in the memory and it is very likely to crash the browser. Looks limiting the size of a file which can be opened is most reasonable. cc @svenefftinge

@akosyakov akosyakov added the help wanted issues meant to be picked up, require help label Sep 9, 2019
@akosyakov akosyakov self-assigned this Sep 10, 2019
@akosyakov akosyakov removed the help wanted issues meant to be picked up, require help label Sep 10, 2019
@svenefftinge
Copy link
Contributor

I think we should have a configurable limit at which the user gets asked before the file is actually opened. I.e. something like:

The file 'bundle.js' is over 10M large. Opening it might take some time and might make the IDE unresponsive. Do you really want to open it?

@akosyakov
Copy link
Member

It should be easy to achieve after #7908

@davemecha
Copy link

This happens all the time.

  • When I want to add an unknown image type (like tif) to my repo and I select the file. => BAM, the diff editor opens.
  • I want to rename an image => BAM, the editor opens
  • I select a large log file in the tree => BAM, the editor opens.

Files that are larger than some (configurable) limit should not be opened directly or maybe on a double click.

@akosyakov
Copy link
Member

What should be the default max file size in the browser context?

@davemecha
Copy link

What should be the default max file size in the browser context?

I think 5 MB would be more than enough in most use cases and when I open a 5 MB file, it still opens quickly without freezing the UI. I also tested with 10 and 15 MB files and this would still be ok but the lag gets bigger.

I'm using Theia IDE with GitPod on a Chromebook.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application editor issues related to the editor filesystem issues related to the filesystem
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants