Skip to content

Commit

Permalink
GUI2/Loading Screen: handle throwing exceptions from the future
Browse files Browse the repository at this point in the history
Despite having exception handing in process(), the loading screen is only designed to handle one exception from the worker.
If such an exception propagates to the worker, the loading screen would rethrow it in process(). It turns out we don't need
an exception_ptr, since the exception would already be stored in the future returned from std::async. Fetching it with get()
causes it to be rethrown at that point, same as before.

Do note we do not want to simply call get() and wait, since that essentially calls future::wait() instead of future::wait_for()
and would halt the main thread while it waits for the worker. Hence, we still manually check its status before fetching the
exception (if any). As a bonus, it means we can use future::valid() as a check for whether the worker is still running in the
dtor, since calling get() sets future::valid() to false. Any premature exit of the loading screen after the worker thread is
created or its exception is handled in the finally process() loop will trigger the safe_exit call.
  • Loading branch information
Vultraz committed Oct 7, 2020
1 parent dba7779 commit 8746993
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 27 deletions.
31 changes: 9 additions & 22 deletions src/gui/dialogs/loading_screen.cpp
Expand Up @@ -111,14 +111,7 @@ void loading_screen::pre_show(window& window)
if(load_func_) {
// Run the load function in its own thread.
try {
worker_result_ = std::async(std::launch::async, [this]() {
try {
load_func_();
} catch(...) {
// TODO: guard this with a mutex.
exception_ = std::current_exception();
}
});
worker_result_ = std::async(std::launch::async, load_func_);
} catch(const std::system_error& e) {
ERR_LS << "Failed to create worker thread: " << e.what() << "\n";
throw;
Expand Down Expand Up @@ -147,9 +140,13 @@ void loading_screen::progress(loading_stage stage)

void loading_screen::process(events::pump_info&)
{
if(!load_func_ || loading_complete()) {
if(exception_) {
std::rethrow_exception(exception_);
using namespace std::chrono_literals;

if(!load_func_ || worker_result_.wait_for(0ms) == std::future_status::ready) {
// The worker returns void, so this is only to handle any exceptions thrown from the worker.
// worker_result_.valid() will return false after.
if(worker_result_.valid()) {
worker_result_.get();
}

get_window()->close();
Expand Down Expand Up @@ -184,12 +181,8 @@ loading_screen::~loading_screen()
* happened because the window was closed, which is not necessarily the case (other
* possibilities might be a 'dialog doesn't fit on screen' exception caused by resizing
* the window).
*
* Another approach might be to add exit points (boost::this_thread::interruption_point())
* to the worker functions (filesystem.cpp, config parsing code, etc.) and then use that
* to end the thread faster.
*/
if(!loading_complete()) {
if(worker_result_.valid()) {
#if defined(_LIBCPP_VERSION) || defined(__MINGW32__)
std::_Exit(0);
#else
Expand All @@ -209,11 +202,5 @@ void loading_screen::display(std::function<void()> f)
}
}

bool loading_screen::loading_complete() const
{
using namespace std::chrono_literals;
return worker_result_.wait_for(0ms) == std::future_status::ready;
}

} // namespace dialogs
} // namespace gui2
5 changes: 0 additions & 5 deletions src/gui/dialogs/loading_screen.hpp
Expand Up @@ -93,9 +93,6 @@ class loading_screen : public modal_dialog, public events::pump_monitor
/** Inherited from events::pump_monitor. */
virtual void process(events::pump_info&) override;

/** Checks whether the worker thread has returned and loading is complete. */
bool loading_complete() const;

/** Callback to handle drawing the progress animation. */
void draw_callback();

Expand All @@ -106,8 +103,6 @@ class loading_screen : public modal_dialog, public events::pump_monitor
std::future<void> worker_result_;
std::unique_ptr<cursor::setter> cursor_setter_;

std::exception_ptr exception_;

label* progress_stage_label_;
label* animation_label_;

Expand Down

0 comments on commit 8746993

Please sign in to comment.