search

microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.04k stars 29.22k forks source link

Consistent OOM Crash while searching short string #133348

Closed uglycoyote closed 2 years ago

uglycoyote commented 3 years ago

Does this issue occur when all extensions are disabled?: Yes

Description

The VSCode search editor (the search that appears as a tab, rather than in the sidebar) nearly always crashes when I type a short search string with a dialog saying "The window has crashed (reason: 'oom')"

This happens on a moderate sized project: ~50k lines of code. Not counting node_modules of course, which I never ever want to search.

My machine has 16GB of ram, and is sitting with 5GB free at the time of this crash. This out of memory crash doesn't seem to actually care about the amount of free system memory. It will happen off a fresh boot.

This has been going on forever, and only after the 1000th time crashing like this did it reach the bar of annoyance where I decided I should complain.

It seems like there are quite a few closed issues about OOM errors but they have all been closed due to inactivity or not enough information. I have attached the crash dump, which hopefully should suffice.

Here are some past issues which might be related

130375

113737

120686

109902

Steps to Reproduce

  1. Press Ctrl+Shift+F to open a search tab in the editor
  2. Type a small phrase of two or three letters, e.g. "in"

image

Crash dump taken with code --disable-extensions --crash-reporter-directory d:/VSCodeCrashes

VSCodeCrash-OOM-Searching.zip

Workarounds

JacksonKearl commented 3 years ago

Are there any particularly large files in the repo that might be causing allocation errors? cc @roblourens

uglycoyote commented 3 years ago

Are there any particularly large files in the repo that might be causing allocation errors?

Not particularly large... find . -printf '%s %p\n' | sort -nr | head -10 reveals that the largest file is 29 Megabytes (some random json file in a cache folder beneath a dependency that I didn't know I had, in node_modules)

So, that's kinda large, but not compared to the amount of free memory I have available on my system.

And, I guess the logical counter-question would be, shouldn't an industrial-strength search tool be able to handle the presence of a few large files without bringing down the application? None of the other applications I use to search (sublime, grep, Notepad++) have run into any difficulty.

If there were a limit on VSCode's abiilty to search large files, a better behaviour would be not to crash, but to detect that the file is over the limit before trying to read it, and just to give me a message in the output saying "didn't search xyz.json, it was too large"

roblourens commented 3 years ago

That's pretty weird. Are you able to share the workspace?

@deepak1556 could you look at the crash dump and check whether it tells us anything useful?

uglycoyote commented 3 years ago

@roblourens apologies, but the project is confidential so I don't have an easily sharable case, but let me know if there's any other diagnostics that I can do on this side.

JacksonKearl commented 3 years ago

Do you have search.maxResults configured at all? You might be able to help us by downloading a public large repo (chromium, linux, perhaps even vscode) and getting a reproducible case there.

uglycoyote commented 3 years ago

Do you have search.maxResults configured at all?

No, I wasn't aware of that setting, so I was using the default of 20000. Setting that to 2000 seems to make it drastically more well behaved, it returns the search results very quickly and so far I haven't been able to get it to give "OOM"

2000 is still a lot of search results to sift through, so that's a perfectly acceptable setting to me. Maybe it's a better default as well since it makes the search much more responsive.

You might be able to help us by downloading a public large repo (chromium, linux, perhaps even vscode) and getting a reproducible case there.

That's a good idea. I cloned vscode, and yes, with the default setting of 20000 search results I reproduced this crash the first time, searching for "aa". It showed a progress bar for 5 seconds and then the same dialog as in my earlier screenshot.

deepak1556 commented 3 years ago

could you look at the crash dump and check whether it tells us anything useful?

This particular OOM was caused by the JS in the renderer, not much useful information from the native stack. It would be good to get a memory profile as next step. Also the user is running 32-bit version of the application, so they are hitting the limit earlier.

The following steps are easier to repro the memory leak,

1) Open the search editor on any large source folder. (I used chromium to capture the attached heap snapshots) 2) Take a baseline snapshot 1 3) Perform a search and take snapshot 2 4) Close the search editor 5) Force GC via devtools 6) Take snapshot 3

Snapshot link (Only MS access)

It looks like search results are held on by the EditorGroupService and are not freed.

Screen Shot 2021-09-27 at 22 07 19
uglycoyote commented 3 years ago

Also the user is running 32-bit version of the application, so they are hitting the limit earlier.

I habitually opt for 64-bit downloaders, so it comes as a surprise to me that I'm running a 32 bit version of VSCode.

I can't see how to verify this from the About page.. it's not very clear as to whether the software is 32 or 64 bit, image

It does say "ia32" next to the OS, but I'm definitely running 64 bit Windows 10. (according to Windows's About page)

image

(maybe ia32 is meant to include Intel 32 and 64 bit processors?)

Assuming that 32 bit is the problem, the out-of-memory failure dialog seems like an excellent place to remind the user that they are severely limiting the ability to address a large amount of RAM by using a 32 bit version.

I suggest a message like: "You have run out of memory. You are running the 32 bit version, and the 64 bit version of the software would be less likely to hit this crash"

That would be a lot more informative that simply "OOM" and -5368707904 . I would definitely have responded to such a message by immediately downloading the 64 bit version.

deepak1556 commented 3 years ago

It does say "ia32" next to the OS, but I'm definitely running 64 bit Windows 10. (according to Windows's About page)

It refers to ia32 runtime of the application and not the OS.

JacksonKearl commented 3 years ago

Thanks for finding that ref @deepak1556 , I'll look into why it's sticking around. In the mean time @uglycoyote were you able to reproduce this in the 64 bit version?

uglycoyote commented 3 years ago

I just tried installing 64-bit VSCode. I verified in the about window that it said "x64" in the OS field, then I loaded the vscode repo and searched. It survived the first two letter search for "in" but when I searched for "aa" it showed the progress bar for about 20-30 seconds and then crashed with OOM -536870904.

So, no... 64 bit does not solve it, it just prolongs the agony

It does seem to have changed the behaviour to where it actually eats up all of my RAM now before crashing.

image

yume-chan commented 3 years ago

ia32 means "Intel Architecture, 32-bit", 64-bit Windows on 64-bit x86 (a.k.a. x64) CPU can still run 32-bit x86 Windows programs.

uglycoyote commented 3 years ago

ia32 means "Intel Architecture, 32-bit", 64-bit Windows on 64-bit x86 (a.k.a. x64) CPU can still run 32-bit x86 Windows programs.

Mostly, I was just confused that this was listed incorrectly to the right of OS in the About dialog, when it was actually trying to indicate whether the vscode software was the 32 vs 64 bit version. That seems like something that ought to be listed separately from the OS description.

Gvozd commented 3 years ago

I also faced this problem for reproduce - need a big one-lined file with a large number of occurrences

For example steps for reproduce:

I have researched this problem and I have a solution PR will be a little later

JacksonKearl commented 2 years ago

Thanks @Gvozd for digging into this and providing a great patch! @uglycoyote, could you help us by verifying if this issue is fixed in the latest insiders?

uglycoyote commented 2 years ago

Thanks, I checked the insiders edition and the crash does not appear, even with the default "max results" of 20000. (I tested this as before, searching "aa" using the search editor, in a local copy of the vscode repo)

It's still pretty lackluster in terms of performance, taking about 30-40 seconds on this machine.

By comparison I can run grep -nir "aa" . in the same folder under WSL2 in about 3 seconds.

So, the crash is gone but I would still honestly suggest changing the default number of search results to 2000 so that the performance is not embarrassingly poor. Even at a max of 2000, it still takes 12 seconds to do the search on this machine.

/verified

JacksonKearl commented 2 years ago

@uglycoyote how does the performance compare to the search view in the side panel? That uses a streaming search, I'd be interested how long it takes for the overall stream to finish

uglycoyote commented 2 years ago

Here are a couple random timings, (these just done with stopwatch app on my phone, so +/- 2 sec accuracy here)

(not sure why, but searching for re seems way faster than searching for aa)