# HG changeset patch # User Daniel J Sebald # Date 1429249120 18000 # Node ID 89d843d6de14da16e3d74778c69b4e95089f51c1 # Parent c6c5cb9c4743aab582a628cc1d169f2562774d26 Add mutex lock to shutdown confirmation for proper thread timing (bug #44751). * main-window.cc (main_window::confirm_shutdown_octave): Lock Qt link mutex before confirming shutdown thereby ensuring link is in sleep state. Unlock mutex after confirming shutdown, then awake all threads waiting on the mutex. * dialog.cc (QUIWidgetCreator::dialog_button_clicked): Block threads by locking mutex before setting dialog_button and dialog_result. Unlock mutex afterward. (QUIWidgetCreator::list_select_finished): Lock mutex before setting list_index and dialog_result. Unlock mutex afterward. (QUIWidgetCreator::input_finished): Lock mutex before setting string_list and dialog_result. Unlock mutex afterward. (QUIWidgetCreator::filedialog_finished): Lock mutex before setting path_name, string_list and dialog_result. Unlock mutex afterward. * libgui/src/dialog.h (QUIWidgetCreator::wait): Remove function and make the QMutex (mutex) and QWaitCondition (waitcondition) available to public. * octave-qt-link.cc (octave_qt_link::octave_qt_link): Move the octave_qt_link object affinity to the main_thread. (octave_qt_link::do_confirm_shutdown): Lock mutex before emitting signal so that GUI thread waits until mutex is free. Unlock the mutex after coming out of sleep state. (octave_qt_link::do_prompt_new_edit_file): Lock the uiwidget_creator mutex before emitting signal and go to sleep after emitting signal. Unlock mutex after mutually shared data has been read. (octave_qt_link::do_message_dialog): Ditto. (octave_qt_link::do_question_dialog): Ditto. (octave_qt_link::do_list_dialog): Ditto. (octave_qt_link::do_input_dialog): Ditto. (octave_qt_link::do_file_dialog): Ditto. (octave_qt_link::do_debug_cd_or_addpath_error): Ditto. diff -r c6c5cb9c4743 -r 89d843d6de14 libgui/src/dialog.cc --- a/libgui/src/dialog.cc Fri Apr 17 18:16:51 2015 -0700 +++ b/libgui/src/dialog.cc Fri Apr 17 00:38:40 2015 -0500 @@ -62,6 +62,9 @@ void QUIWidgetCreator::dialog_button_clicked (QAbstractButton *button) { + // Wait for link thread to go to sleep state. + mutex.lock (); + // Store the value so that builtin functions can retrieve. if (button) dialog_button = button->text (); @@ -69,6 +72,8 @@ // The value should always be 1 for the Octave functions. dialog_result = 1; + mutex.unlock (); + // Wake up Octave process so that it continues. waitcondition.wakeAll (); } @@ -78,10 +83,15 @@ QUIWidgetCreator::list_select_finished (const QIntList& selected, int button_pressed) { + // Wait for link thread to go to sleep state. + mutex.lock (); + // Store the value so that builtin functions can retrieve. *list_index = selected; dialog_result = button_pressed; + mutex.unlock (); + // Wake up Octave process so that it continues. waitcondition.wakeAll (); } @@ -90,10 +100,15 @@ void QUIWidgetCreator::input_finished (const QStringList& input, int button_pressed) { + // Wait for link thread to go to sleep state. + mutex.lock (); + // Store the value so that builtin functions can retrieve. *string_list = input; dialog_result = button_pressed; + mutex.unlock (); + // Wake up Octave process so that it continues. waitcondition.wakeAll (); } @@ -102,11 +117,16 @@ QUIWidgetCreator::filedialog_finished (const QStringList& files, const QString& path, int filterindex) { + // Wait for link thread to go to sleep state. + mutex.lock (); + // Store the value so that builtin functions can retrieve. *string_list = files; dialog_result = filterindex; *path_name = path; + mutex.unlock (); + // Wake up Octave process so that it continues. waitcondition.wakeAll (); } diff -r c6c5cb9c4743 -r 89d843d6de14 libgui/src/dialog.h --- a/libgui/src/dialog.h Fri Apr 17 18:16:51 2015 -0700 +++ b/libgui/src/dialog.h Fri Apr 17 00:38:40 2015 -0500 @@ -114,11 +114,11 @@ const QString *get_dialog_path (void) { return path_name; } - void wait (void) - { - // Wait while the user is responding to message box. - waitcondition.wait (&mutex); - } + // GUI objects cannot be accessed in the non-GUI thread. However, + // signals can be sent to slots across threads with proper + // synchronization. Hence, the use of QWaitCondition. + QMutex mutex; + QWaitCondition waitcondition; signals: @@ -159,13 +159,6 @@ QString *path_name; - // GUI objects cannot be accessed in the non-GUI thread. However, - // signals can be sent to slots across threads with proper - // synchronization. Hence, the use of QWaitCondition. - - QMutex mutex; - - QWaitCondition waitcondition; }; extern QUIWidgetCreator uiwidget_creator; diff -r c6c5cb9c4743 -r 89d843d6de14 libgui/src/main-window.cc --- a/libgui/src/main-window.cc Fri Apr 17 18:16:51 2015 -0700 +++ b/libgui/src/main-window.cc Fri Apr 17 00:38:40 2015 -0500 @@ -794,10 +794,16 @@ #endif } + // Wait for link thread to go to sleep state. + _octave_qt_link->mutex.lock (); + _octave_qt_link->shutdown_confirmation (closenow); + _octave_qt_link->mutex.unlock (); + // Awake the worker thread so that it continues shutting down (or not). _octave_qt_link->waitcondition.wakeAll (); + } void diff -r c6c5cb9c4743 -r 89d843d6de14 libgui/src/octave-qt-link.cc --- a/libgui/src/octave-qt-link.cc Fri Apr 17 18:16:51 2015 -0700 +++ b/libgui/src/octave-qt-link.cc Fri Apr 17 00:38:40 2015 -0500 @@ -56,6 +56,10 @@ command_interpreter->moveToThread (main_thread); main_thread->start (); + + // Move this link object affinity to the same thread as the + // interpreter to ensure signals/slots to the GUI are queued. + moveToThread (main_thread); } octave_qt_link::~octave_qt_link (void) { } @@ -69,12 +73,18 @@ bool octave_qt_link::do_confirm_shutdown (void) { + // Lock the mutex before emitting signal. + mutex.lock (); + emit confirm_shutdown_signal (); // Wait while the GUI shuts down. waitcondition.wait (&mutex); - // The GUI has sent a signal and the process has been awakened. + // The GUI has sent a signal and the thread has been awakened. + + mutex.unlock (); + return _shutdown_confirm_result; } @@ -110,6 +120,9 @@ role << "YesRole" << "RejectRole"; btn << tr ("Create") << tr ("Cancel"); + // Lock mutex before signaling. + uiwidget_creator.mutex.lock (); + uiwidget_creator.signal_dialog ( tr ("File\n%1\ndoes not exist. Do you want to create it?"). arg (QDir::currentPath () + QDir::separator () @@ -117,10 +130,14 @@ tr ("Octave Editor"), "quest", btn, tr ("Create"), role ); // Wait while the user is responding to message box. - uiwidget_creator.wait (); - // The GUI has sent a signal and the process has been awakened. + uiwidget_creator.waitcondition.wait (&uiwidget_creator.mutex); + + // The GUI has sent a signal and the thread has been awakened. + QString answer = uiwidget_creator.get_dialog_button (); + uiwidget_creator.mutex.unlock (); + return (answer == tr ("Create")); } @@ -129,6 +146,9 @@ const std::string& msg, const std::string& title) { + // Lock mutex before signaling. + uiwidget_creator.mutex.lock (); + uiwidget_creator.signal_dialog (QString::fromStdString (msg), QString::fromStdString (title), QString::fromStdString (dlg), @@ -136,10 +156,15 @@ QStringList ()); // Wait while the user is responding to message box. - uiwidget_creator.wait (); + uiwidget_creator.waitcondition.wait (&uiwidget_creator.mutex); + + // The GUI has sent a signal and the thread has been awakened. - // The GUI has sent a signal and the process has been awakened. - return uiwidget_creator.get_dialog_result (); + int answer = uiwidget_creator.get_dialog_result (); + + uiwidget_creator.mutex.unlock (); + + return answer; } std::string @@ -160,6 +185,9 @@ btn << QString::fromStdString (btn2); btn << QString::fromStdString (btn3); + // Lock mutex before signaling. + uiwidget_creator.mutex.lock (); + uiwidget_creator.signal_dialog (QString::fromStdString (msg), QString::fromStdString (title), "quest", @@ -168,10 +196,15 @@ role); // Wait while the user is responding to message box. - uiwidget_creator.wait (); + uiwidget_creator.waitcondition.wait (&uiwidget_creator.mutex); + + // The GUI has sent a signal and the thread has been awakened. - // The GUI has sent a signal and the process has been awakened. - return uiwidget_creator.get_dialog_button ().toStdString (); + std::string answer = uiwidget_creator.get_dialog_button ().toStdString (); + + uiwidget_creator.mutex.unlock (); + + return answer; } static QStringList @@ -232,6 +265,9 @@ const std::string& ok_string, const std::string& cancel_string) { + // Lock mutex before signaling. + uiwidget_creator.mutex.lock (); + uiwidget_creator.signal_listview (make_qstring_list (list), QString::fromStdString (mode), width, height, @@ -242,12 +278,15 @@ QString::fromStdString (cancel_string)); // Wait while the user is responding to message box. - uiwidget_creator.wait (); + uiwidget_creator.waitcondition.wait (&uiwidget_creator.mutex); - // The GUI has sent a signal and the process has been awakened. + // The GUI has sent a signal and the thread has been awakened. + const QIntList *selected = uiwidget_creator.get_list_index (); int ok = uiwidget_creator.get_dialog_result (); + uiwidget_creator.mutex.unlock (); + return std::pair, int> (selected->toStdList (), ok); } @@ -260,6 +299,9 @@ { std::list retval; + // Lock mutex before signaling. + uiwidget_creator.mutex.lock (); + uiwidget_creator.signal_inputlayout (make_qstring_list (prompt), QString::fromStdString (title), QFloatList::fromStdList (nr), @@ -267,11 +309,14 @@ make_qstring_list (defaults)); // Wait while the user is responding to message box. - uiwidget_creator.wait (); + uiwidget_creator.waitcondition.wait (&uiwidget_creator.mutex); + + // The GUI has sent a signal and the thread has been awakened. - // The GUI has sent a signal and the process has been awakened. const QStringList *inputLine = uiwidget_creator.get_string_list (); + uiwidget_creator.mutex.unlock (); + for (QStringList::const_iterator it = inputLine->begin (); it != inputLine->end (); it++) { @@ -290,6 +335,9 @@ { std::list retval; + // Lock mutex before signaling. + uiwidget_creator.mutex.lock (); + uiwidget_creator.signal_filedialog (make_filter_list (filter), QString::fromStdString (title), QString::fromStdString (filename), @@ -297,7 +345,9 @@ QString::fromStdString (multimode)); // Wait while the user is responding to dialog. - uiwidget_creator.wait (); + uiwidget_creator.waitcondition.wait (&uiwidget_creator.mutex); + + // The GUI has sent a signal and the thread has been awakened. // Add all the file dialog results to a string list. const QStringList *inputLine = uiwidget_creator.get_string_list (); @@ -310,6 +360,8 @@ retval.push_back ((QString ("%1").arg ( uiwidget_creator.get_dialog_result ())).toStdString ()); + uiwidget_creator.mutex.unlock (); + return retval; } @@ -346,13 +398,20 @@ btn << cancel_txt; role << "AcceptRole"; + // Lock mutex before signaling. + uiwidget_creator.mutex.lock (); + uiwidget_creator.signal_dialog (msg, title, "quest", btn, cancel_txt, role); // Wait while the user is responding to message box. - uiwidget_creator.wait (); + uiwidget_creator.waitcondition.wait (&uiwidget_creator.mutex); + + // The GUI has sent a signal and the thread has been awakened. QString result = uiwidget_creator.get_dialog_button (); + uiwidget_creator.mutex.unlock (); + if (result == cd_txt) retval = 1; else if (result == addpath_txt)