Skip to content

Commit

Permalink
Fix blocking of shutdown on Windows
Browse files Browse the repository at this point in the history
Some code was creating windows in the renderer process. These windows are
included in Windows' enumeration of a renderer's threads at log off/shutdown.
Renderers do not have a UI message loop (aren't pumped) and so don't respond
to messages that Windows sends during shutdown. As a result, they appear hung,
and shutdown is blocked.

Remove two locations that were creating windows, and add a DCHECK that the
main renderer thread does not create windows.

BUG=230122,236031,236039

Review URL: https://chromiumcodereview.appspot.com/14305025

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@197922 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
scottmg@chromium.org committed May 2, 2013
1 parent 0ed307a commit 2dd68fd
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 1 deletion.
7 changes: 7 additions & 0 deletions base/power_monitor/power_monitor_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ bool PowerMonitor::IsBatteryPower() {

PowerMonitor::PowerMessageWindow::PowerMessageWindow()
: instance_(NULL), message_hwnd_(NULL) {
if (MessageLoop::current()->type() != MessageLoop::TYPE_UI) {
// Creating this window in (e.g.) a renderer inhibits shutdown on Windows.
// See http://crbug.com/230122. TODO(vandebo): http://crbug.com/236031
DLOG(ERROR)
<< "Cannot create windows on non-UI thread, power monitor disabled!";
return;
}
WNDCLASSEX window_class;
base::win::InitializeWindowClass(
kWindowClassName,
Expand Down
18 changes: 18 additions & 0 deletions content/renderer/renderer_main_platform_delegate_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ void InitExitInterceptions() {
base::win::SetAbortBehaviorForCrashReporting();
}

#if !defined(NDEBUG)
LRESULT CALLBACK WindowsHookCBT(int code, WPARAM w_param, LPARAM l_param) {
CHECK_NE(code, HCBT_CREATEWND)
<< "Should not be creating windows in the renderer!";
return CallNextHookEx(NULL, code, w_param, l_param);
}
#endif // !NDEBUG


} // namespace

RendererMainPlatformDelegate::RendererMainPlatformDelegate(
Expand All @@ -59,6 +68,15 @@ RendererMainPlatformDelegate::~RendererMainPlatformDelegate() {
}

void RendererMainPlatformDelegate::PlatformInitialize() {
#if !defined(NDEBUG)
// Install a check that we're not creating windows in the renderer. See
// http://crbug.com/230122 for background. TODO(scottmg): Ideally this would
// check all threads in the renderer, but it currently only checks the main
// thread.
PCHECK(
SetWindowsHookEx(WH_CBT, WindowsHookCBT, NULL, ::GetCurrentThreadId()));
#endif // !NDEBUG

InitExitInterceptions();

// Be mindful of what resources you acquire here. They can be used by
Expand Down
14 changes: 13 additions & 1 deletion ui/base/win/singleton_hwnd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "ui/base/win/singleton_hwnd.h"

#include "base/memory/singleton.h"
#include "base/message_loop.h"

namespace ui {

Expand All @@ -14,12 +15,23 @@ SingletonHwnd* SingletonHwnd::GetInstance() {
}

void SingletonHwnd::AddObserver(Observer* observer) {
if (!hwnd())

if (!hwnd()) {
if (!MessageLoop::current() ||
MessageLoop::current()->type() != MessageLoop::TYPE_UI) {
// Creating this window in (e.g.) a renderer inhibits shutdown on
// Windows. See http://crbug.com/230122 and http://crbug.com/236039.
DLOG(ERROR) << "Cannot create windows on non-UI thread!";
return;
}
WindowImpl::Init(NULL, gfx::Rect());
}
observer_list_.AddObserver(observer);
}

void SingletonHwnd::RemoveObserver(Observer* observer) {
if (!hwnd())
return;
observer_list_.RemoveObserver(observer);
}

Expand Down

0 comments on commit 2dd68fd

Please sign in to comment.