Skip to content

Commit

Permalink
Fixed|Client|libcore|libappfw: More graceful fatal errors
Browse files Browse the repository at this point in the history
A fatal error should not crash the engine, just pop up an error message.

Client's BusyRunner was ignoring the busy worker abort notification.
Now it causes the busy worker to be instantly killed.

While a fatal error message is displayed the main game is window
is just hidden, not deleted. Deleting everything while the objects
are in use is not a great idea.

IssueID #2251
  • Loading branch information
skyjake committed Jun 19, 2017
1 parent aec1e12 commit 174dcab
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 20 deletions.
39 changes: 27 additions & 12 deletions doomsday/apps/client/src/busyrunner.cpp
Expand Up @@ -40,10 +40,13 @@
#include <de/Config>
#include <de/GLInfo>
#include <de/Log>
#include <de/Loop>

#include <QEventLoop>
#include <atomic>

using namespace de;

static bool animatedTransitionActive(int busyMode)
{
return (!novideo && !isDedicated && !netGame && !(busyMode & BUSYF_STARTUP) &&
Expand All @@ -59,6 +62,7 @@ DENG2_PIMPL_NOREF(BusyRunner)
, DENG2_OBSERVES(BusyMode, Beginning)
, DENG2_OBSERVES(BusyMode, End)
, DENG2_OBSERVES(BusyMode, TaskWillStart)
, DENG2_OBSERVES(BusyMode, Abort)
{
QEventLoop *eventLoop = nullptr;

Expand All @@ -74,14 +78,15 @@ DENG2_PIMPL_NOREF(BusyRunner)
busy().audienceForBeginning() += this;
busy().audienceForEnd() += this;
busy().audienceForTaskWillStart() += this;
busy().audienceForAbort() += this;
}

~Impl()
{
busy().setTaskRunner(nullptr);
}

void busyModeWillBegin(BusyTask &firstTask)
void busyModeWillBegin(BusyTask &firstTask) override
{
if (auto *fader = ClientWindow::main().contentFade())
{
Expand All @@ -105,7 +110,7 @@ DENG2_PIMPL_NOREF(BusyRunner)
ClientWindow::main().setMode(ClientWindow::Busy);
}

void busyModeEnded()
void busyModeEnded() override
{
DD_ResetTimer();

Expand All @@ -119,7 +124,7 @@ DENG2_PIMPL_NOREF(BusyRunner)
ClientWindow::main().setMode(ClientWindow::Normal);
}

void busyTaskWillStart(BusyTask &task)
void busyTaskWillStart(BusyTask &task) override
{
// Is the worker updating its progress?
if (task.maxProgress > 0)
Expand All @@ -128,6 +133,16 @@ DENG2_PIMPL_NOREF(BusyRunner)
}
}

void busyModeAborted(String const &) override
{
Loop::mainCall([this] ()
{
qDebug() << "[BusyRunner] Killing the worker!";
Thread_KillAbnormally(busyThread);
exitEventLoop();
});
}

/**
* Exits the busy mode event loop. Called in the main thread, does not return
* until the worker thread is stopped.
Expand Down Expand Up @@ -189,11 +204,11 @@ BusyRunner::Result BusyRunner::runTask(BusyTask *task)
// Let's get busy!
BusyVisual_PrepareResources();

de::ProgressWidget &prog = ClientWindow::main().busy().progress();
ProgressWidget &prog = ClientWindow::main().busy().progress();
prog.show((task->mode & BUSYF_PROGRESS_BAR) != 0);
prog.setText(task->name);
prog.setMode(task->mode & BUSYF_ACTIVITY? de::ProgressWidget::Indefinite :
de::ProgressWidget::Ranged);
prog.setMode(task->mode & BUSYF_ACTIVITY? ProgressWidget::Indefinite :
ProgressWidget::Ranged);

// Start the busy worker thread, which will process the task in the
// background while we keep the user occupied with nice animations.
Expand Down Expand Up @@ -221,15 +236,15 @@ BusyRunner::Result BusyRunner::runTask(BusyTask *task)
* event loop used during busy mode. Toggling the swap interval off and
* back on appears to be a valid workaround.
*/
de::Loop::timer(0.1, [] () {
Loop::timer(0.1, [] () {
ClientWindow::main().glActivate();
de::GLInfo::setSwapInterval(0);
GLInfo::setSwapInterval(0);
ClientWindow::main().glDone();
});
de::Loop::timer(0.5, [] () {
Loop::timer(0.5, [] () {
ClientWindow::main().glActivate();
if (de::Config::get().getb("window.main.vsync")) {
de::GLInfo::setSwapInterval(1);
if (Config::get().getb("window.main.vsync")) {
GLInfo::setSwapInterval(1);
}
ClientWindow::main().glDone();
});
Expand Down Expand Up @@ -330,7 +345,7 @@ void BusyMode_FreezeGameForBusyMode(void)
// This is only possible from the main thread.
if (ClientWindow::mainExists() &&
DoomsdayApp::app().busyMode().taskRunner() &&
de::App::inMainThread())
App::inMainThread())
{
#if !defined (DENG_MOBILE)
ClientWindow::main().busy().renderTransitionFrame();
Expand Down
11 changes: 8 additions & 3 deletions doomsday/apps/client/src/dd_main.cpp
Expand Up @@ -563,6 +563,8 @@ void App_Error(char const *error, ...)

void App_AbnormalShutdown(char const *message)
{
DENG2_ASSERT_IN_MAIN_THREAD();

#ifdef __CLIENT__
// This is a crash landing, better be safe than sorry.
DoomsdayApp::app().busyMode().setTaskRunner(nullptr);
Expand All @@ -579,7 +581,10 @@ void App_AbnormalShutdown(char const *message)
// that the app's event loop is running normally while we show the native
// message box below -- if the app windows are not hidden/closed, they might
// receive draw events.
ClientApp::windowSystem().closeAll();
ClientApp::windowSystem().forAll([] (BaseWindow *win) {
win->hide();
return LoopContinue;
});
#endif

if (message) // Only show if a message given.
Expand Down Expand Up @@ -1699,13 +1704,13 @@ dint DD_GetInteger(dint ddvalue)
{
return 0;
}

// Update pointers.
if (ddvalue == DD_NUMSOUNDS)
{
ddValues[ddvalue].readPtr = &DED_Definitions()->sounds.count.num;
}

if (ddValues[ddvalue].readPtr == 0)
{
return 0;
Expand Down
2 changes: 1 addition & 1 deletion doomsday/apps/client/src/ui/widgets/gamewidget.cpp
Expand Up @@ -281,7 +281,7 @@ void GameWidget::update()
{
GuiWidget::update();

if (isDisabled() || BusyMode_Active()) return;
if (DoomsdayApp::app().isShuttingDown() || isDisabled() || BusyMode_Active()) return;

// We may be performing GL operations.
ClientWindow::main().glActivate();
Expand Down
2 changes: 2 additions & 0 deletions doomsday/sdk/libappfw/include/de/framework/windowsystem.h
Expand Up @@ -95,6 +95,8 @@ class LIBAPPFW_PUBLIC WindowSystem : public System
*/
BaseWindow *find(String const &id) const;

LoopResult forAll(std::function<LoopResult (BaseWindow *)> func);

/**
* Closes all windows, including the main window.
*/
Expand Down
4 changes: 4 additions & 0 deletions doomsday/sdk/libappfw/src/guirootwidget.cpp
Expand Up @@ -437,7 +437,9 @@ void GuiRootWidget::update()

void GuiRootWidget::draw()
{
#if defined (DENG_MOBILE)
DENG2_GUARD(this);
#endif

DENG2_ASSERT_IN_RENDER_THREAD();

Expand Down Expand Up @@ -470,7 +472,9 @@ void GuiRootWidget::draw()

void GuiRootWidget::drawUntil(Widget &until)
{
#if defined (DENG_MOBILE)
DENG2_GUARD(this);
#endif

DENG2_ASSERT_IN_RENDER_THREAD();

Expand Down
14 changes: 13 additions & 1 deletion doomsday/sdk/libappfw/src/windowsystem.cpp
Expand Up @@ -103,8 +103,20 @@ BaseWindow *WindowSystem::find(String const &id) const
return 0;
}

LoopResult WindowSystem::forAll(std::function<LoopResult (BaseWindow *)> func)
{
for (BaseWindow *win : d->windows)
{
if (auto result = func(win))
{
return result;
}
}
return LoopContinue;
}

void WindowSystem::closeAll()
{
{
closingAllWindows();

qDeleteAll(d->windows.values());
Expand Down
14 changes: 12 additions & 2 deletions doomsday/sdk/libcore/src/widgets/rootwidget.cpp
Expand Up @@ -97,8 +97,10 @@ Rule const &RootWidget::viewHeight() const

void RootWidget::setViewSize(Size const &size)
{
#if defined (DENG_MOBILE)
DENG2_GUARD(this);

#endif

d->viewRect->setInput(Rule::Right, Constu(size.x));
d->viewRect->setInput(Rule::Bottom, Constu(size.y));

Expand Down Expand Up @@ -141,27 +143,35 @@ Widget *RootWidget::focus() const

void RootWidget::initialize()
{
#if defined (DENG_MOBILE)
DENG2_GUARD(this);
#endif
notifyTree(&Widget::initialize);
}

void RootWidget::update()
{
#if defined (DENG_MOBILE)
DENG2_GUARD(this);
#endif
notifyTree(&Widget::update);
}

void RootWidget::draw()
{
#if defined (DENG_MOBILE)
DENG2_GUARD(this);
#endif
notifyTree(notifyArgsForDraw());
Rule::markRulesValid(); // All done for this frame.
}

bool RootWidget::processEvent(Event const &event)
{
#if defined (DENG_MOBILE)
DENG2_GUARD(this);

#endif

// Focus is only for the keyboard.
if (event.isKey() && focus())
{
Expand Down
4 changes: 3 additions & 1 deletion doomsday/sdk/liblegacy/include/de/concurrency.h
Expand Up @@ -82,7 +82,7 @@ protected slots:
* @param startpos Executes while the thread is running. When the function exists,
* the thread stops.
* @param parm Parameter given to the thread function.
* @param terminationFunc Callback function that is called from the worker thread
* @param terminationFunc Callback function that is called from the worker thread
* right before it exits. The callback is given the exit status
* of the thread as a parameter.
* @return Thread handle.
Expand Down Expand Up @@ -115,6 +115,8 @@ DENG_PUBLIC thread_t Sys_StartThread(int (*startpos)(void *), void *parm,

DENG_PUBLIC void Thread_Sleep(int milliseconds);

DENG_PUBLIC void Thread_KillAbnormally(thread_t handle);

/**
* Wait for a thread to stop. If the thread does not stop after @a timeoutMs,
* it will be forcibly terminated.
Expand Down

0 comments on commit 174dcab

Please sign in to comment.