Skip to content

Commit

Permalink
fix: shutdown after message loop is ready (#16672)
Browse files Browse the repository at this point in the history
  • Loading branch information
trop[bot] authored and codebytere committed Feb 1, 2019
1 parent afa684a commit d1d0efb
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 38 deletions.
2 changes: 1 addition & 1 deletion atom/browser/atom_browser_main_parts.cc
Expand Up @@ -455,7 +455,7 @@ bool AtomBrowserMainParts::MainMessageLoopRun(int* result_code) {

void AtomBrowserMainParts::PreDefaultMainMessageLoopRun(
base::OnceClosure quit_closure) {
Browser::SetMainMessageLoopQuitClosure(std::move(quit_closure));
Browser::Get()->SetMainMessageLoopQuitClosure(std::move(quit_closure));
}

void AtomBrowserMainParts::PostMainMessageLoopStart() {
Expand Down
17 changes: 9 additions & 8 deletions atom/browser/browser.cc
Expand Up @@ -25,9 +25,6 @@

namespace atom {

// Null until/unless the default main message loop is running.
base::NoDestructor<base::OnceClosure> g_quit_main_message_loop;

Browser::LoginItemSettings::LoginItemSettings() = default;
Browser::LoginItemSettings::~LoginItemSettings() = default;
Browser::LoginItemSettings::LoginItemSettings(const LoginItemSettings& other) =
Expand Down Expand Up @@ -95,11 +92,12 @@ void Browser::Shutdown() {
for (BrowserObserver& observer : observers_)
observer.OnQuit();

if (*g_quit_main_message_loop) {
std::move(*g_quit_main_message_loop).Run();
if (quit_main_message_loop_) {
std::move(quit_main_message_loop_).Run();
} else {
// There is no message loop available so we are in early stage.
exit(0);
// There is no message loop available so we are in early stage, wait until
// the quit_main_message_loop_ is available.
// Exiting now would leave defunct processes behind.
}
}

Expand Down Expand Up @@ -196,7 +194,10 @@ void Browser::PreMainMessageLoopRun() {
}

void Browser::SetMainMessageLoopQuitClosure(base::OnceClosure quit_closure) {
*g_quit_main_message_loop = std::move(quit_closure);
if (is_shutdown_)
std::move(quit_closure).Run();
else
quit_main_message_loop_ = std::move(quit_closure);
}

void Browser::NotifyAndShutdown() {
Expand Down
5 changes: 4 additions & 1 deletion atom/browser/browser.h
Expand Up @@ -244,7 +244,7 @@ class Browser : public WindowListObserver {

// Stores the supplied |quit_closure|, to be run when the last Browser
// instance is destroyed.
static void SetMainMessageLoopQuitClosure(base::OnceClosure quit_closure);
void SetMainMessageLoopQuitClosure(base::OnceClosure quit_closure);

void AddObserver(BrowserObserver* obs) { observers_.AddObserver(obs); }

Expand Down Expand Up @@ -287,6 +287,9 @@ class Browser : public WindowListObserver {
// The browser is being shutdown.
bool is_shutdown_ = false;

// Null until/unless the default main message loop is running.
base::OnceClosure quit_main_message_loop_;

int badge_count_ = 0;

util::Promise* ready_promise_ = nullptr;
Expand Down
7 changes: 0 additions & 7 deletions spec/api-app-spec.js
Expand Up @@ -1222,13 +1222,6 @@ describe('default behavior', () => {
})

describe('window-all-closed', () => {
before(function () {
// FIXME(jkleinsc): Test is consistently failing on Windows 32 bit.
if (process.arch === 'ia32') {
this.skip()
}
})

it('quits when the app does not handle the event', async () => {
const result = await runTestApp('window-all-closed')
expect(result).to.equal(false)
Expand Down
7 changes: 0 additions & 7 deletions spec/api-browser-view-spec.js
Expand Up @@ -228,13 +228,6 @@ describe('BrowserView module', () => {
})

describe('new BrowserView()', () => {
before(function () {
// FIXME(jkleinsc): Test is consistently failing on Windows 32 bit.
if (process.arch === 'ia32') {
this.skip()
}
})

it('does not crash on exit', async () => {
const appPath = path.join(fixtures, 'api', 'leak-exit-browserview.js')
const electronPath = remote.getGlobal('process').execPath
Expand Down
7 changes: 0 additions & 7 deletions spec/api-web-contents-spec.js
Expand Up @@ -893,13 +893,6 @@ describe('webContents module', () => {
})

describe('create()', () => {
before(function () {
// FIXME(jkleinsc): Test is consistently failing on Windows 32 bit.
if (process.arch === 'ia32') {
this.skip()
}
})

it('does not crash on exit', async () => {
const appPath = path.join(__dirname, 'fixtures', 'api', 'leak-exit-webcontents.js')
const electronPath = remote.getGlobal('process').execPath
Expand Down
7 changes: 0 additions & 7 deletions spec/api-web-contents-view-spec.js
Expand Up @@ -34,13 +34,6 @@ describe('WebContentsView', () => {
})

describe('new WebContentsView()', () => {
before(function () {
// FIXME(jkleinsc): Test is consistently failing on Windows 32 bit.
if (process.arch === 'ia32') {
this.skip()
}
})

it('does not crash on exit', async () => {
const appPath = path.join(__dirname, 'fixtures', 'api', 'leak-exit-webcontentsview.js')
const electronPath = remote.getGlobal('process').execPath
Expand Down

0 comments on commit d1d0efb

Please sign in to comment.