From 1e7829db97d5134336d6f9e4961c1cfd1d1bf9d5 Mon Sep 17 00:00:00 2001 From: Zack Weinberg Date: Wed, 20 Aug 2014 14:38:45 -0400 Subject: [PATCH] Fix crash on exit if pages have been closed (#12482) Regression from #12431: the loop to clear all surviving pages neglects to check for entries in `m_pages` which have already been closed, fully destructed, and therefore replaced by NULL pointers. --- src/phantom.cpp | 6 ++++-- test/regression/issue12482.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 test/regression/issue12482.js diff --git a/src/phantom.cpp b/src/phantom.cpp index 88634e28..003714f4 100644 --- a/src/phantom.cpp +++ b/src/phantom.cpp @@ -493,8 +493,7 @@ void Phantom::clearCookies() // private: void Phantom::doExit(int code) { - if (m_config.debug()) - { + if (m_config.debug()) { Utils::cleanupFromDebug(); } @@ -502,6 +501,9 @@ void Phantom::doExit(int code) m_terminated = true; m_returnValue = code; foreach (QPointer page, m_pages) { + if (!page) + continue; + // stop processing of JavaScript code by loading a blank page page->mainFrame()->setUrl(QUrl(QStringLiteral("about:blank"))); // delay deletion into the event loop, direct deletion can trigger crashes diff --git a/test/regression/issue12482.js b/test/regression/issue12482.js new file mode 100644 index 00000000..2fc0ad93 --- /dev/null +++ b/test/regression/issue12482.js @@ -0,0 +1,30 @@ +// https://github.com/ariya/phantomjs/issues/12482 +// regression caused by fix for +// https://github.com/ariya/phantomjs/issues/12431 + +var webpage = require('webpage'); +var pages = [ + webpage.create(), + webpage.create(), + webpage.create() +]; + +var yet_to_load = pages.length; +function loadHook (status) { + if (status !== "success") + console.log("FAIL: status = " + status); + if (--yet_to_load == 0) { + pages[1].close(); + setTimeout(function(){ + phantom.exit(0); + console.log("FAIL: should not get here"); + }, 50); + } +} + +for (var i = 0; i < pages.length; i++) { + pages[i].onConsoleMessage = function(msg) { console.log(msg); }; + pages[i].open( + "data:text/html,", loadHook); +}