-
Notifications
You must be signed in to change notification settings - Fork 7.6k
[CEF 2171] Crash on quit. #7683
Comments
Below is the call stack from the release build
|
@RaymondLim Does File > Exit crash as well? |
I didn't try it and now I'm back to master. But I did debug it yesterday and it is our exit command that is quitting immediately instead of waiting for JS callback's return. I believe message handling/routing changes in cef 1750 are causing this behavior. |
I don't know much about the message handling/routing. I do know that the Can you try to dSym the stack? The CEF functions are nameless. |
After realizing the crash is in JS side, I used bisect to locate between sprint-36 and sprint-37. It turns out that it is my commit 4eee5ea that implements the new view states migration from the old pref. Apparently, the crash is the result of the shell code tearing down everything while the pref manager is still handling some fileIO by writing out some view states. @dangoor Can you suggest how we can return a promise from pref manager to the code that handles the quit command? |
@RaymondLim
How does that sound? |
@dangoor I am unable to trap this crash in debugger, so I have attempted to implement your suggestion in PR #7930 . Please take a look and let me know if I understood correctly -- it seems to prevent crash sometimes, but not always. Reminder: this is a CEF 1750 bug, so be sure to use the |
@redmunds I tried in your branch and still crash on exit with Ctrl+Q all the time if I made no changes in any document. If I made a change in a document, then your code seems to work correctly, but not in the build w/o your changes. As you said, I can't trap the crash in debugger. If I do, then I won't be able to use File > Exit or Ctrl+Q to quit any more. It seems like we have dangling promises after debugging. |
Marking Release 41 |
With PR #7930, the problem is greatly reduced and then seems to be a problem with Quiting while JS Code Hints engine is still initializing. See my notes here. |
@JeffryBooher Here's the callstack from the remaining crash:
|
Thanks I'll pass it on On Jun 16, 2014, at 8:24 PM, "Randy Edmunds" <[email protected]mailto:[email protected]> wrote: @JeffryBooherhttps://github.com/JeffryBooher Here's the callstack from the remaining crash:
Reply to this email directly or view it on GitHubhttps://github.com//issues/7683#issuecomment-46263517. |
Needs Review. If this only occurs on shutdown after startup it doesn't feel like a case that we should hold up the release for. I suggest Medium. Added Tracking and removed Fix in Progress labels. |
It's a little broader than that -- it can also happen if you shutdown after switching projects or selecting a file in a new folder (that JS Code Hints has not yet processed files for). |
@peterflynn My comment above should answer your question #1 and I'll be closing the terminate tern worker pull request. For question #2, I think the answer is yes. I did replace |
@peterflynn @RaymondLim About the web worker crash, when we were looking at the crash on quit we got a callstack different than the one reported above. And that is why we were proposing two fixes. If the crash(or set of crashes) goes out with clearing callback_maps_, then there is no need for us to terminate the web worker explicitly. We figured that not terminating the web worker and quitting crashes only with Brackets. Instead of loading Brackets index.html we loaded a web page which starts a webworker. Now without terminating this web worker if we quit Brackets, Brackets was crashing. However we could not repro the same with cefclient. |
This change involves some code changes got from code review comments from (#487) pull request.
Removing cef and tracking labels since we're not waiting any fix from CEF. |
Setting to FBNC since the fix has been merged |
This crash is driving me crazy. Now I can still reproduce it with jeff/cef-2171 branch and below is the call stack.
@nethip Do you want to take a look and see you can figure out what's causing this? You may need to try several times to get one crash though. I got this call stack on the sixth try. |
@RaymondLim I guess one relatively "cheap" test we could try now would be similar to what we were looking at before:
|
I got 2 more crashes today with same call stack as @RaymondLim |
@peterflynn Can't get any crash after disabling JS Code Hints. So @nethip claim of tern worker crash seems to be right. So I ran his simple worker thread and got the following call stack.
@nethip I'll be restoring my pull request to terminate tern worker and see whether I still can reproduce the crash or not. |
@peterflynn @redmunds @RaymondLim As @RaymondLim mentioned, we fixed this crash by terminating the tern worker. I am sure this will go off with tern worker shutdown. We could consistently reproduce this crash on my Win laptop. @peterflynn Spawning any web worker is causing this crash on quit. Like what @RaymondLim did, We also spawned a sample web worker instead of tern-worker.js and even then Brackets was crashing on quit. In fact instead of loading index.html we loaded a sample HTML which spawns a web worker. Even for that case Brackets was crashing. I will see if I can get any more pointers from CEF side as we could very well have an extension which might spawn a web worker and not explicitly terminate the same on quit.. For now we can go with the tern-worker terminate solution. |
I had restored pull request #10020 and tried to crash it on quit for a long while with no success. So I think we should go with it in order to update to the newer cef. |
@RaymondLim This CL looks fine to me. |
Merged #10020 |
Launch Brackets and press Ctrl+Q to quit. You will get a crash with the message saying "Brackets has stopped working."
Workaround: click on 'X' button in the title bar to quit.
The text was updated successfully, but these errors were encountered: