Hi Dirk,
On Dienstag, 31. Oktober 2017 20:41:32 CET Dirk Hohndel wrote:
> On Tue, Oct 31, 2017 at 08:10:32PM +0100, Berthold Stoeger wrote:
> > To be honest, apart from the errors mentioned above, I'm not even sure
> > that
> > the idea behind the code is sound. If you want, I can send an
> > updated/cleaned up patch, but I have a bad feeling with this one. There
> > probably should at least be a non-varargs version.
>
> The idea that the thread collects its messages and that the caller of the
> thread then initiates their display when it is done seems sound to me.
>
> I'd really like a version that's good enough to use in 4.7.2 - and I'm
> open to further refinement once that's released.
I still think the approach is fundamentally broken/brittle. You have to
identify which calls are made from the GUI thread and which are not. Who knows
if I got the right report_error()s? What about errors that happen only in few
cases?
So in the attachment is a different approach: don't output errors if not in
the main thread. Someone who generates a thread is responsible of flushing the
errors later. Sounds much less brittle to me.
This means that the membuffer in errorhelper.c has to be protected by a mutex
or something (not an expert in threads). But that race was there before,
wasn't it?
What do you think?
Berthold
diff --git a/desktop-widgets/downloadfromdivecomputer.cpp b/desktop-widgets/downloadfromdivecomputer.cpp
index 637f6034..b875b4a7 100644
--- a/desktop-widgets/downloadfromdivecomputer.cpp
+++ b/desktop-widgets/downloadfromdivecomputer.cpp
@@ -220,6 +220,10 @@ void DownloadFromDCWidget::updateState(states state)
// got an error
else if (state == ERROR) {
timer->stop();
+
+ // Show messages that worker thread produced.
+ MainWindow::instance()->showErrors();
+
QMessageBox::critical(this, TITLE_OR_TEXT(tr("Error"), thread.error), QMessageBox::Ok);
markChildrenAsEnabled();
progress_bar_text = "";
diff --git a/desktop-widgets/mainwindow.cpp b/desktop-widgets/mainwindow.cpp
index 95495b66..2119e375 100644
--- a/desktop-widgets/mainwindow.cpp
+++ b/desktop-widgets/mainwindow.cpp
@@ -88,12 +88,25 @@ MainWindow *MainWindow::m_Instance = NULL;
extern "C" void showErrorFromC()
{
+ // Show errors only if we are running in the gui thread.
+ // If we're in the gui thread, let errors accumulate.
+ if (QThread::currentThread() != QCoreApplication::instance()->thread()) {
+ return;
+ }
+
MainWindow *mainwindow = MainWindow::instance();
if (mainwindow) {
mainwindow->getNotificationWidget()->showNotification(get_error_string(), KMessageWidget::Error);
}
}
+void MainWindow::showErrors()
+{
+ const char *error = get_error_string();
+ if (error && error[0]) {
+ getNotificationWidget()->showNotification(error, KMessageWidget::Error);
+ }
+}
MainWindow::MainWindow() : QMainWindow(),
actionNextDive(0),
diff --git a/desktop-widgets/mainwindow.h b/desktop-widgets/mainwindow.h
index edd3fb0c..50590aa2 100644
--- a/desktop-widgets/mainwindow.h
+++ b/desktop-widgets/mainwindow.h
@@ -89,6 +89,10 @@ public:
void enableDisableCloudActions();
void setCheckedActionFilterTags(bool checked);
+ // Shows errors that have accumulated.
+ // Must be called from GUI thread.
+ void showErrors();
+
private
slots:
/* file menu action */
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
