Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Fix for crash on quit w.r.t any web worker still running during quit. ( ... #490

Merged
merged 2 commits into from
Dec 9, 2014

Conversation

nethip
Copy link
Contributor

@nethip nethip commented Dec 9, 2014

...[CEF 2171] Crash on quit. #7683 )

This fixes the crash with web workers still running while quitting the
main Brackets app. CefQuitMessageLoop() needs to be called for the CEF
message loop to quit gracefully. This change involves removing handling
of WM_DESTROY and doing the same in ClientHandler::OnBeforeClose().

… ( [CEF 2171] Crash on quit. #7683 )

This fixes the crash with web workers still running while quitting the
main Brackets app. CefQuitMessageLoop() needs to be called for the CEF
message loop to quit gracefully. This change involves removing handling
of WM_DESTROY and doing the same in ClientHandler::OnBeforeClose().
@nethip
Copy link
Contributor Author

nethip commented Dec 9, 2014

@peterflynn @RaymondLim @JeffryBooher @redmunds Please review this change. This change involves changing the destruction sequence to match to that of cefclient's.

…st app calls it on other platforms and we should do the same thing on quit.
@RaymondLim
Copy link
Contributor

@nethip Looks good except for your restriction to Windows. I removed that restriction since Cefclient test app is also calling CefQuitMessageLoop() on other platforms and according to documentation in CefLifeSpanHandler.h, it is the right way to quit. Merging now.

RaymondLim added a commit that referenced this pull request Dec 9, 2014
Fix for crash on quit w.r.t any web worker still running during quit. ( ...
@RaymondLim RaymondLim merged commit 68b7139 into jeff/cef_2171 Dec 9, 2014
@RaymondLim RaymondLim deleted the prashant_2171_crash_fix branch December 9, 2014 22:37
@nethip
Copy link
Contributor Author

nethip commented Dec 10, 2014

@RaymondLim I agree with you on CefQuitMessageLoop() need to be called for all the platforms. I did a quick search and found out that for Linux we are calling CefQuitMessageLoop() on SIGTERM(inside TerminationSignalHandler). So does it make sense to exclude CefQuitMessageLoop() inside OnBeforeClose() for Linux platform?

@RaymondLim
Copy link
Contributor

@nethip Sure, can you create a new pull request with that change? @ingorichter can review it as he is working on Linux for quit issue.

Update: It may also be fixing the issue @ingorichter is investigating.

@nethip
Copy link
Contributor Author

nethip commented Dec 10, 2014

@RaymondLim I have made the necessary changes and created a pull request for the same.

@ingorichter Could you please review the following pull request and let me know if it makes sense. I have tested this change on master(as I could not build jeff/cef_2171) and things seem to be working fine there.

#491

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants