Skip to content

Commit

Permalink
#2937 fix: possible crash on certain systems at startup after the scr…
Browse files Browse the repository at this point in the history
…ipt update check
  • Loading branch information
pbek committed Jan 1, 2024
1 parent 0335f6c commit 226d171
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 19 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# QOwnNotes Changelog

## 24.1.1
- a crash on certain systems at startup after the script update check was fixed
(for [#2937](https://github.com/pbek/QOwnNotes/issues/2937))

## 24.1.0
- the script update check at startup now runs in a separate thread to not block
the application startup (for [#2934](https://github.com/pbek/QOwnNotes/issues/2934))
Expand Down
6 changes: 5 additions & 1 deletion src/dialogs/scriptrepositorydialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ bool ScriptRepositoryDialog::loadScriptRepositoryMetaData() {
QUrl("https://github.com/qownnotes/scripts/releases/download/metadata-index/index.json");

int statusCode;
// This should have a 10-second timeout
auto arr = Utils::Misc::downloadUrlWithStatusCode(url, statusCode);

if (statusCode != 200) {
Expand Down Expand Up @@ -158,8 +159,11 @@ void ScriptRepositoryDialog::searchForUpdatesForScripts(const QList<Script>& scr
continue;
}

if (!scriptUpdateFound) {
emit updateFound();
}

scriptUpdateFound = true;
emit updateFound();
addScriptTreeWidgetItem(scriptInfoJson);
ui->selectFrame->show();
}
Expand Down
39 changes: 21 additions & 18 deletions src/mainwindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10951,49 +10951,51 @@ void MainWindow::on_actionCheck_for_script_updates_triggered() {
*/
void MainWindow::automaticScriptUpdateCheck() {
#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
// Sqlite isn't available in a separate thread, so we need to fetch the scripts before that
auto scripts = Script::fetchAll();

// We don't need to check for updates if there are no scripts
if (scripts.isEmpty()) {
return;
}

_scriptUpdateFound = false;
auto *dialog = new ScriptRepositoryDialog(this, true);

// Show a message once if no script update were found
// We need to do that in a slot, because you can't use a timer in a separate thread
QObject::connect(dialog, &ScriptRepositoryDialog::noUpdateFound, this, [this]() {
QObject::connect(dialog, &ScriptRepositoryDialog::noUpdateFound, this, [this, dialog]() {
showStatusBarMessage(tr("No script updates were found"), 3000);
delete(dialog);
});

// Show a dialog once if a script update was found
QObject::connect(dialog, &ScriptRepositoryDialog::updateFound, this, [this]() {
QObject::connect(dialog, &ScriptRepositoryDialog::updateFound, this, [this, dialog]() {
// We only want to run this once
if (_scriptUpdateFound) {
return;
}

_scriptUpdateFound = true;
showStatusBarMessage(tr("A script update was found!"), 4000);
delete(dialog);

// Open the update question dialog in another thread so the dialog
// can be deleted meanwhile
QTimer::singleShot(100, this, [this]() {
if (Utils::Gui::question(this, tr("Script updates"),
tr("Updates to your scripts were found in the script "
"repository! Do you want to update them?"),
"auto-script-update") == QMessageBox::Yes) {
on_actionCheck_for_script_updates_triggered();
}
});
if (Utils::Gui::question(this, tr("Script updates"),
tr("Updates to your scripts were found in the script "
"repository! Do you want to update them?"),
"auto-script-update") == QMessageBox::Yes) {
on_actionCheck_for_script_updates_triggered();
}
});

// Sqlite isn't available in a separate thread, so we need to fetch the scripts before that
auto scripts = Script::fetchAll();

// Search for script updates in the background after the "updateFound" signal was connected
// We must not do a `delete (dialog)` in the new thread, that was causing a crash on some systems
// See https://github.com/pbek/QOwnNotes/issues/2937
QFuture<void> future = QtConcurrent::run([this, dialog, scripts]() {
qDebug() << "start searchForUpdates";
dialog->searchForUpdatesForScripts(scripts);
qDebug() << "done searchForUpdates";

// Delete the dialog afterward
delete (dialog);

if (_scriptUpdateFound) {
_scriptUpdateFound = false;
}
Expand Down Expand Up @@ -11061,6 +11063,7 @@ void MainWindow::on_noteTreeWidget_itemDoubleClicked(QTreeWidgetItem *item, int
Q_UNUSED(item)
Q_UNUSED(column)

// call a script hook that a new note was double-clicked
const bool hookFound =
ScriptingService::instance()->callHandleNoteDoubleClickedHook(&currentNote);

Expand Down

0 comments on commit 226d171

Please sign in to comment.