iOS: trying 2 fixes in gui2::execute_timer() and gui2::timer_callback(): timer thread safety or tooltips.

timer callback can invoke whole stack of the windows. Don't lock the timers forever.
This commit is contained in:
Victor Sergienko 2017-11-23 16:01:10 -08:00 committed by Jyrki Vesterinen
parent 12bd7d34b6
commit 8668b7fcb7
2 changed files with 57 additions and 24 deletions

View file

@ -344,6 +344,14 @@ void mouse_motion::mouse_leave()
void mouse_motion::start_hover_timer(widget* widget, const point& coordinate)
{
assert(widget);
#ifdef __IPHONEOS__
// Guessing a crash location in a nasty stack in gui2::execute_timer.
// Either this or a long-touch menu.
// Remove this when the crash in gui2::execute_timer() and gui2::timer_callback() is gone and try again.
return;
#endif
stop_hover_timer();
if(hover_shown_ || !widget->wants_mouse_hover()) {

View file

@ -20,6 +20,7 @@
#include <SDL_timer.h>
#include <map>
#include <mutex>
namespace gui2
{
@ -53,6 +54,8 @@ static std::map<std::size_t, timer>& get_timers()
*/
static std::size_t executing_id = 0;
std::mutex timers_mutex;
/** Did somebody try to remove the timer during its execution? */
static bool executing_id_removed = false;
@ -93,11 +96,17 @@ extern "C" {
static uint32_t timer_callback(uint32_t, void* id)
{
DBG_GUI_E << "Pushing timer event in queue.\n";
// iTunes still reports a couple of crashes here. Cannot see a problem yet.
std::map<std::size_t, timer>::iterator itor
= get_timers().find(reinterpret_cast<std::size_t>(id));
if(itor == get_timers().end()) {
return 0;
Uint32 result;
{
std::lock_guard<std::mutex> lock(timers_mutex);
auto itor = get_timers().find(reinterpret_cast<std::size_t>(id));
if(itor == get_timers().end()) {
return 0;
}
result = itor->second.interval;
}
SDL_Event event;
@ -109,7 +118,7 @@ static uint32_t timer_callback(uint32_t, void* id)
SDL_PushEvent(&event);
return itor->second.interval;
return result;
}
} // extern "C"
@ -122,13 +131,18 @@ std::size_t add_timer(const uint32_t interval,
DBG_GUI_E << "Adding timer.\n";
do {
++next_timer_id;
} while(next_timer_id == 0 || get_timers().find(next_timer_id) != get_timers().end());
timer timer;
timer.sdl_id = SDL_AddTimer(
interval, timer_callback, reinterpret_cast<void*>(next_timer_id));
{
std::lock_guard<std::mutex> lock(timers_mutex);
do {
++next_timer_id;
} while(next_timer_id == 0 || get_timers().count(next_timer_id) > 0);
timer.sdl_id = SDL_AddTimer(
interval, timer_callback, reinterpret_cast<void*>(next_timer_id));
}
if(timer.sdl_id == 0) {
WRN_GUI_E << "Failed to create an sdl timer." << std::endl;
return 0;
@ -140,7 +154,11 @@ std::size_t add_timer(const uint32_t interval,
timer.callback = callback;
get_timers().emplace(next_timer_id, timer);
{
std::lock_guard<std::mutex> lock(timers_mutex);
get_timers().emplace(next_timer_id, timer);
}
DBG_GUI_E << "Added timer " << next_timer_id << ".\n";
return next_timer_id;
@ -150,7 +168,9 @@ bool remove_timer(const std::size_t id)
{
DBG_GUI_E << "Removing timer " << id << ".\n";
std::map<std::size_t, timer>::iterator itor = get_timers().find(id);
std::lock_guard<std::mutex> lock(timers_mutex);
auto itor = get_timers().find(id);
if(itor == get_timers().end()) {
LOG_GUI_E << "Can't remove timer since it no longer exists.\n";
return false;
@ -181,20 +201,25 @@ bool execute_timer(const std::size_t id)
{
DBG_GUI_E << "Executing timer " << id << ".\n";
std::map<std::size_t, timer>::iterator itor = get_timers().find(id);
if(itor == get_timers().end()) {
LOG_GUI_E << "Can't execute timer since it no longer exists.\n";
return false;
}
std::function<void(size_t)> callback = nullptr;
{
executor executor(id);
itor->second.callback(id);
std::lock_guard<std::mutex> lock(timers_mutex);
std::map<size_t, timer>::iterator itor = get_timers().find(id);
if(itor == get_timers().end()) {
LOG_GUI_E << "Can't execute timer since it no longer exists.\n";
return false;
}
callback = itor->second.callback;
if(itor->second.interval == 0) {
get_timers().erase(itor);
}
}
if(!executing_id_removed && itor->second.interval == 0) {
get_timers().erase(itor);
}
callback(id);
return true;
}