-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Intermittent WSOD when editing brackets Gruntfile.js #4549
Comments
I was able to reproduce on mac and windows. If it didn't reproduce right away, making simple edits like backspacing to retype the line seemed to do the trick too. |
I just reproed with these exact steps :( |
Here's the stack:
|
To me for now. I got some tips from @dangoor on how we could try to track down another workaround. |
This is a small point, but this crash also reproduces without the original workaround. (I just wanted to check that to make sure that this isn't actually a regression caused by the original workaround.) |
On the assumption that this is another V8 crash in Acorn, I tried to narrow down the crash by writing a Brackets extension that simply runs acorn on every .js file in a folder (recursively), and pointed it at the Brackets folder. Unfortunately, I couldn't get it to crash. I also couldn't get it to crash on polymer.min.js (with the version of Acorn that doesn't have @dangoor's workaround), though, so there must be something else that I need to do in order to trigger even the original crash. Tern doesn't pass any special options to Acorn, though, so it's hard to imagine what that would be. |
@gruehle mentioned that the original crash happened when rescanning the same file after small changes a lot of times, so I tried having Acorn scan each file multiple times in a row, but that still doesn't trigger the original crash in polymer or in the brackets codebase. My original reason for trying this approach is that I can't easily use @dangoor suggested hacking JS code hints so they don't run Tern as a worker, but I couldn't find a quick way to do that. (I looked at a web worker polyfill that simulates the worker with |
It might not take that long to hack the worker and the host in |
@jasonsanjose if you feel like looking into that at some point before tomorrow (unlikely since it's late in the day) let me know. |
I've found steps that will reproduce this in master, so it's not related to any of the other changes in this branch:
If I type the function wrapper instead of pasting it before typing the call to I'm not sure that tells us anything :), but at least we have a slightly reduced repro case in master. |
Try the |
Crash doesn't appear to occur in that branch, as expected, though I've only tried my smaller repro case so far. |
I now have a branch that runs Tern in the main thread, but I can't get the crash to occur in that branch (either Jason's original or my reduced version) :( So it's possible that this is actually a different problem that's related to the worker. It could actually be a bug in workers, or it could just be that because the asynchronicity is different, something else changes that avoids the crash. It would be interesting to try to reproduce the original crash @dangoor's workaround fixes in this branch, but I don't know how to do that (there was a link in that bug that appears to have been a recipe for reproducing it, but the link was removed). |
Progress: the immediate cause of the crash is src/utils/KeyEvent.js. If I remove that from a copy of the Brackets project, I can't reproduce the crash. UPDATE: Red herring. It seems like I can reproduce it even if that file is removed now. |
I haven't reproduced the crash in the |
Closing since the upgrade to CEF has landed in master and @jasonsanjose has confirmed the crash no longer occurs. |
I'm going to reopen this since we're still tracking which CEF version we're going to ship. I've been working on finding a workaround for this. It is difficult because the crashing code is in a web worker and if we try to use the debugger on the worker, the code doesn't hit the optimizer (and therefore doesn't crash). And the worker doesn't have What I have seen consistently is that the worker receives a
The handling of WorkingSetView-test.js appears to run fine. The worker does not send any messages back to the main thread after that, though. My theory is that the worker is dying before it actually has a chance to post the messages back. After WorkingSetView-test is sent into the worker, the main thread loads up a handful of additional files, the first of which is My guess is that it's crashing when parsing bootstrap-dropdown.js, but I haven't yet figured out a way to narrow it down more than that. Since this crash is so reproducible, perhaps the next step is to just start selectively forcing functions in Acorn to not optimize (through the |
Yup, that's all very consistent with what I was seeing on Wednesday. |
I have a workaround for this crash in the dangoor/v8-crash/redux branch. I have added I did some performance tests this morning and found that when parsing polymer.min.js, acorn took on average about 13% longer to parse (median was 9% more) after adding all of the |
That's not to bad, how certain are we that this addresses the bulk of the potential crasher? |
So far, all of the crashes that I've seen with the sprint 27 shell (including the one in this bug) have been caused by v8's optimizer on Acorn. It is certainly possible for v8's optimizer to fail on some other piece of code, but it appears that there's something specific in Acorn that triggers this bug. Barring reports of some other crash, this workaround seems to do the trick. |
FBNC to @jasonsanjose |
Confirmed. Closing. |
Do we want to keep this open until we merge the new CEF? (I guess in some sense it was already closed due to the workaround...) |
Seems fine to me to close it given that we have both a new CEF that appears to work and a workaround if it doesn't. |
jasonsanjose/grunt-hack-setup
(branched from master 830c83c as of this morning 7/23)if (grunt.file.exists(path)) {
I've had varying success reproducing this crash. It might depend on how quickly you begin typing after Brackets starts up and/or whether or not the Gruntfile.js was already in the working set.
The text was updated successfully, but these errors were encountered: