Keep three crash-recovery snapshots instead of overwriting one

Discussion #598, reviving PR #654. The crash-recovery backup written
every 20 minutes went to a single file. If the project was already in a
bad state when a backup ran, that bad state replaced the only recovery
copy.

QETProject now writes the backups in turn to three KAutoSaveFile slots
(BackupGenerations), so one bad write only replaces the oldest
snapshot. Scope is crash recovery only; the opt-in autosave is
unchanged.

After a crash, the recovery prompt groups the snapshots by project and
offers one row per project with a list to pick the snapshot to reopen,
newest selected by default. The snapshots not picked are deleted.

Ported onto current master: writeBackup() keeps the "skip if nothing
changed" check (bugtracker #273) and offerBackupFiles() keeps its place
after the stale-file filter and before the crash report (#901).

Tested with the backup interval shortened to 4 s (test build only):
after three changes, master holds one recovery file, overwritten each
time; this branch holds three, with 4, 5 and 6 folios. After killing
QET, the prompt lists the project; picking the oldest snapshot reopens
4 folios, the default reopens 6. Twice each. ctest 34/34 with and
without KDE Frameworks; the carried-over KAutoSaveFile test passes in
the nokde build.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cmake/qet_compilation_vars.cmake

@@ -752,6 +752,8 @@ set(QET_SRC_FILES
   ${QET_DIR}/sources/ui/diagrampropertieseditordockwidget.h
   ${QET_DIR}/sources/ui/backupdialog.cpp
   ${QET_DIR}/sources/ui/backupdialog.h
+  ${QET_DIR}/sources/ui/backuprestoredialog.cpp
+  ${QET_DIR}/sources/ui/backuprestoredialog.h
   ${QET_DIR}/sources/ui/dialogwaiting.cpp
   ${QET_DIR}/sources/ui/dialogwaiting.h
   ${QET_DIR}/sources/ui/duplicateoffsetdialog.cpp

sources/qetapp.cpp

@@ -55,6 +55,7 @@
 #include <iostream>
 #define QUOTE(x) STRINGIFY(x)
 #define STRINGIFY(x) #x
+#include <QFileInfo>
 #include <QFontDatabase>
 #include <QProcessEnvironment>
 #include <QRegularExpression>
@@ -65,6 +66,9 @@
 #else
 #	include <KAutoSaveFile>
 #endif
+#include "ui/backuprestoredialog.h"
+
+#include <algorithm>
 
 #ifdef QET_ALLOW_OVERRIDE_CED_OPTION
 QString QETApp::m_overrided_common_elements_dir = QString();
@@ -2839,48 +2843,47 @@ void QETApp::checkBackupFiles()
 /**
 	@brief QETApp::offerBackupFiles
 	Ask whether to reopen the recovery files left by a previous run, and
-	open or discard them accordingly.
+	open or discard them accordingly. A project can leave several recovery
+	files, one per snapshot (@see QETProject::writeBackup): they are grouped
+	by project, and the user picks which one to reopen, the newest by default.
 	@param stale_files : the recovery files to offer
 */
 void QETApp::offerBackupFiles(const QList<KAutoSaveFile *> &stale_files)
 {
-	QString text;
-	if(stale_files.size() == 1) {
-		text.append(tr("<b>Le fichier de restauration suivant a été trouvé,<br>"
-					   "Voulez-vous l'ouvrir ?</b><br>"));
-	} else {
-		text.append(tr("<b>Les fichiers de restauration suivant on été trouvé,<br>"
-					   "Voulez-vous les ouvrir ?</b><br>"));
+		//Group the snapshots by the project they recover, newest first.
+	QHash<QString, QList<KAutoSaveFile *>> groups;
+	for (KAutoSaveFile *kasf : stale_files) {
+		groups[kasf->managedFile().path()].append(kasf);
 	}
-	for(const KAutoSaveFile *kasf : stale_files)
-	{
-#	ifdef Q_OS_WIN
-	//Remove the first character '/' before the name of the drive
-	text.append("<br>" + kasf->managedFile().path().remove(0,1));
-#	else
-	text.append("<br>" + kasf->managedFile().path());
-#	endif
+	for (auto &snapshots : groups) {
+		std::sort(snapshots.begin(), snapshots.end(),
+			[](KAutoSaveFile *a, KAutoSaveFile *b) {
+				return QFileInfo(*a).lastModified()
+					 > QFileInfo(*b).lastModified();
+			});
 	}
 
-	//Open backup file
-	if (QET::QetMessageBox::question(nullptr,
-					 tr("Fichier de restauration"),
-					 text,
-					 QMessageBox::Ok
-					 |QMessageBox::Cancel
-					 )
-			== QMessageBox::Ok)
+	BackupRestoreDialog dialog(groups, nullptr);
+	if (dialog.exec() == QDialog::Accepted)
 	{
+			//The snapshots not picked are no longer needed.
+		for (KAutoSaveFile *discarded : dialog.discardedFiles())
+		{
+			discarded->open(QIODevice::ReadWrite);
+			delete discarded;
+		}
+
+		const QList<KAutoSaveFile *> to_open = dialog.selectedFiles();
 		//If there are open editors, find those that are visible
 		if (diagramEditors().count())
 		{
 			diagramEditors().first()->setVisible(true);
-			diagramEditors().first()->openBackupFiles(stale_files);
+			diagramEditors().first()->openBackupFiles(to_open);
 		}
 		else
 		{
 			QETDiagramEditor *editor = new QETDiagramEditor();
-			editor->openBackupFiles(stale_files);
+			editor->openBackupFiles(to_open);
 		}
 	}
 	else //Clear backup file

sources/qetproject.cpp

@@ -205,7 +205,7 @@ QETProject::QETProject(KAutoSaveFile *backup, QObject *parent) :
 QETProject::~QETProject()
 {
 		//Wait for any in-flight async crash-recovery backup to finish: the worker
-		//writes through &m_backup_file, a member that would otherwise be destroyed
+		//writes through m_backup_files, a member that would otherwise be destroyed
 		//under it (issue #492).
 	m_backup_future.waitForFinished();
 
@@ -562,12 +562,16 @@ void QETProject::setFilePath(const QString &filepath)
 	if (filepath == m_file_path) {
 		return;
 	}
-		//Don't close/re-point the backup file while a backup is still writing it.
+		//Don't close/re-point the backup files while a backup is still writing one.
 	m_backup_future.waitForFinished();
-	if (m_backup_file.isOpen()) {
-		m_backup_file.close();
+	const QUrl managed_file = QUrl::fromLocalFile(filepath);
+	for (auto &backup_file : m_backup_files) {
+		if (backup_file.isOpen()) {
+			backup_file.close();
+		}
+		backup_file.setManagedFile(managed_file);
 	}
-	m_backup_file.setManagedFile(QUrl::fromLocalFile(filepath));
+	m_next_backup_slot = 0;
 	m_file_path = filepath;
 
 	QFileInfo fi(m_file_path);
@@ -2337,14 +2341,17 @@ void QETProject::detachDiagram(Diagram *diagram)
 
 /**
 	@brief QETProject::writeBackup
-	Write a backup file of this project, in the case that QET crash
+	Write a backup file of this project, in the case that QET crash.
+	The snapshots are written in turn to m_backup_files, so a write made
+	while the project is already in a bad state only replaces the oldest
+	snapshot, and the earlier ones are still there to recover from.
 */
 void QETProject::writeBackup()
 {
 	if (!m_backup_enabled)
 		return;
 		//Don't launch a new backup while the previous one is still writing:
-		//both would write through &m_backup_file on different threads.
+		//both could write through the same m_backup_files slot on different threads.
 	if (m_backup_future.isRunning())
 		return;
 		//toXml() walks the whole project on the GUI thread, which freezes
@@ -2357,8 +2364,10 @@ void QETProject::writeBackup()
 		//Qt5-style QtConcurrent::run(function, reference-args) call did not
 		//survive the Qt6 API change, a lambda behaves identically on both.
 	QDomDocument xml_project(toXml());
-	m_backup_future = QtConcurrent::run([this, xml_project]() mutable {
-		return QET::writeToFile(xml_project, &m_backup_file, nullptr);
+	KAutoSaveFile *target = &m_backup_files[m_next_backup_slot];
+	m_next_backup_slot = (m_next_backup_slot + 1) % BackupGenerations;
+	m_backup_future = QtConcurrent::run([target, xml_project]() mutable {
+		return QET::writeToFile(xml_project, target, nullptr);
 	});
 }
 

sources/qetproject.h

@@ -39,6 +39,8 @@
 #include <QSet>
 #include <QFuture>
 
+#include <array>
+
 class Diagram;
 class ElementsLocation;
 class QETResult;
@@ -128,6 +130,10 @@ class QETProject : public QObject
 		/// process can destroy the project before the write finishes (crash).
 		static void setBackupEnabled(bool enabled);
 
+		/// Number of crash-recovery snapshots kept per project, written in
+		/// turn by writeBackup(), so one bad write cannot replace the only copy
+		static constexpr int BackupGenerations = 3;
+
 			///DEFAULT PROPERTIES
 		BorderProperties defaultBorderProperties() const;
 		void             setDefaultBorderProperties(const BorderProperties &);
@@ -366,7 +372,9 @@ class QETProject : public QObject
 		QTimer m_save_backup_timer,
 			   m_autosave_timer;
 		QFuture<bool> m_backup_future;
-		KAutoSaveFile m_backup_file;
+			/// Crash-recovery snapshots, written in turn by writeBackup()
+		std::array<KAutoSaveFile, BackupGenerations> m_backup_files;
+		int m_next_backup_slot = 0;
 		QUuid m_uuid = QUuid::createUuid();
 		QHash<QString, int> m_derived_uuid_keys;
 		QSet<QUuid> m_saved_item_uuids;	//symbol and wire uuids the file carries, see derivedItemUuid()

sources/ui/backuprestoredialog.cpp

@@ -0,0 +1,142 @@
+/*
+	Copyright 2006-2026 The QElectroTech Team
+	This file is part of QElectroTech.
+
+	QElectroTech is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 2 of the License, or
+	(at your option) any later version.
+
+	QElectroTech is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with QElectroTech.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "backuprestoredialog.h"
+
+#ifdef BUILD_WITHOUT_KF
+#	include "nokde/kautosavefile.h"
+#else
+#	include <KAutoSaveFile>
+#endif
+
+#include <QComboBox>
+#include <QDateTime>
+#include <QDialogButtonBox>
+#include <QFileInfo>
+#include <QGridLayout>
+#include <QLabel>
+#include <QLocale>
+#include <QVBoxLayout>
+
+/**
+	@brief BackupRestoreDialog::BackupRestoreDialog
+	@param groups : managed project path -> its recovery generations
+	(newest-first)
+	@param parent : parent widget
+*/
+BackupRestoreDialog::BackupRestoreDialog(
+	const QHash<QString, QList<KAutoSaveFile *>> &groups,
+	QWidget *parent) :
+	QDialog(parent),
+	m_groups(groups)
+{
+	setWindowTitle(tr("Fichiers de restauration", "window title"));
+
+	auto main_layout = new QVBoxLayout(this);
+
+	auto intro = new QLabel(
+		tr("<b>Des fichiers de restauration ont été trouvés,<br>"
+		   "voulez-vous les ouvrir ?</b><br>"
+		   "Pour un projet ayant plusieurs versions de restauration, "
+		   "la plus récente est sélectionnée par défaut.",
+		   "dialog message"));
+	intro->setWordWrap(true);
+	main_layout->addWidget(intro);
+
+	auto grid = new QGridLayout();
+	int row = 0;
+	for (auto it = m_groups.constBegin(); it != m_groups.constEnd(); ++it)
+	{
+		const QString &path = it.key();
+		const QList<KAutoSaveFile *> &generations = it.value();
+
+#	ifdef Q_OS_WIN
+		QString display_path = path;
+		display_path.remove(0, 1);
+#	else
+		const QString &display_path = path;
+#	endif
+		grid->addWidget(new QLabel(display_path), row, 0);
+
+		auto combo = new QComboBox(this);
+		for (int i = 0; i < generations.size(); ++i)
+		{
+			const QDateTime modified =
+				QFileInfo(*generations.at(i)).lastModified();
+			QString label = QLocale::system().toString(
+				modified, QLocale::ShortFormat);
+			if (i == 0) {
+				label = tr("%1 (la plus récente)", "recovery generation label")
+							.arg(label);
+			}
+			combo->addItem(label);
+		}
+		combo->setEnabled(generations.size() > 1);
+		grid->addWidget(combo, row, 1);
+
+		m_combo_for_path.insert(path, combo);
+		++row;
+	}
+	main_layout->addLayout(grid);
+
+	auto buttons = new QDialogButtonBox(
+		QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this);
+	connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept);
+	connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject);
+	main_layout->addWidget(buttons);
+}
+
+/**
+	@brief BackupRestoreDialog::~BackupRestoreDialog
+*/
+BackupRestoreDialog::~BackupRestoreDialog() = default;
+
+/**
+	@return one recovery generation per project, matching the combo box
+	selection (the newest generation, unless the user picked another).
+*/
+QList<KAutoSaveFile *> BackupRestoreDialog::selectedFiles() const
+{
+	QList<KAutoSaveFile *> selected;
+	for (auto it = m_groups.constBegin(); it != m_groups.constEnd(); ++it)
+	{
+		const int index = m_combo_for_path.value(it.key())->currentIndex();
+		selected << it.value().at(index);
+	}
+	return selected;
+}
+
+/**
+	@return every recovery generation the user did not pick; the caller
+	should release and delete these.
+*/
+QList<KAutoSaveFile *> BackupRestoreDialog::discardedFiles() const
+{
+	QList<KAutoSaveFile *> discarded;
+	for (auto it = m_groups.constBegin(); it != m_groups.constEnd(); ++it)
+	{
+		const int index = m_combo_for_path.value(it.key())->currentIndex();
+		const QList<KAutoSaveFile *> &generations = it.value();
+		for (int i = 0; i < generations.size(); ++i) {
+			if (i != index) {
+				discarded << generations.at(i);
+			}
+		}
+	}
+	return discarded;
+}

sources/ui/backuprestoredialog.h

@@ -0,0 +1,60 @@
+/*
+	Copyright 2006-2026 The QElectroTech Team
+	This file is part of QElectroTech.
+
+	QElectroTech is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 2 of the License, or
+	(at your option) any later version.
+
+	QElectroTech is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with QElectroTech.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#ifndef BACKUPRESTOREDIALOG_H
+#define BACKUPRESTOREDIALOG_H
+
+#include <QDialog>
+#include <QHash>
+#include <QList>
+#include <QString>
+
+class KAutoSaveFile;
+class QComboBox;
+
+/**
+	@brief Lets the user pick, per crashed project, which recovery
+	generation to reopen when the crash-recovery rotation
+	(@see QETProject::writeBackup) left more than one snapshot behind.
+*/
+class BackupRestoreDialog : public QDialog
+{
+	Q_OBJECT
+
+	public:
+		/// @param groups : managed project file path -> its stale recovery
+		/// generations, each list already sorted newest-first. Ownership of
+		/// the KAutoSaveFile objects stays with the caller.
+		explicit BackupRestoreDialog(
+			const QHash<QString, QList<KAutoSaveFile *>> &groups,
+			QWidget *parent = nullptr);
+		~BackupRestoreDialog() override;
+
+		/// Valid once accepted: one entry per project, the chosen generation
+		/// (defaults to the most recent one).
+		QList<KAutoSaveFile *> selectedFiles() const;
+		/// The generations the user did not pick; the caller should discard
+		/// (release + delete) these.
+		QList<KAutoSaveFile *> discardedFiles() const;
+
+	private:
+		QHash<QString, QList<KAutoSaveFile *>> m_groups;
+		QHash<QString, QComboBox *> m_combo_for_path;
+};
+
+#endif // BACKUPRESTOREDIALOG_H

tests/catch/src/kautosavefile_test.cpp

@@ -3,14 +3,18 @@
 #include <catch2/catch.hpp>
 
 #include <QCoreApplication>
+#include <QDateTime>
 #include <QFile>
 #include <QFileInfo>
 #include <QTemporaryDir>
 #include <QUrl>
 
+#include <algorithm>
 #include <memory>
+#include <vector>
 
 #ifdef Q_OS_UNIX
+#include <ctime>
 #include <signal.h>
 #include <sys/types.h>
 #include <sys/wait.h>
@@ -106,3 +110,104 @@ TEST_CASE("Qt-only KAutoSaveFile recovers stale files", "[nokde][autosave]")
 	CHECK_FALSE(QFile::exists(lock_file_name));
 #endif
 }
+
+TEST_CASE("Multiple KAutoSaveFile generations for one managed file are all "
+		  "found stale after a crash", "[nokde][autosave]")
+{
+#ifndef Q_OS_UNIX
+	SUCCEED("crash-style stale lock test is Unix-only");
+#else
+		//Simulates QETProject's rotating crash-recovery generations
+		//(BackupGenerations snapshots written round-robin): several
+		//KAutoSaveFile instances sharing one managed file, alive at once.
+	QTemporaryDir data_home;
+	REQUIRE(data_home.isValid());
+
+	qputenv("XDG_DATA_HOME", QFile::encodeName(data_home.path()));
+	QCoreApplication::setOrganizationName(QStringLiteral("QElectroTech"));
+	QCoreApplication::setApplicationName(
+		QStringLiteral("KAutoSaveFileGenerationsTest"));
+
+	const auto managed_path = data_home.filePath(QStringLiteral("project.qet"));
+	QFile managed_file(managed_path);
+	REQUIRE(managed_file.open(QIODevice::WriteOnly | QIODevice::Text));
+	REQUIRE(managed_file.write("<project/>\n") > 0);
+	managed_file.close();
+
+	constexpr int generations = 3;
+
+	int ready_pipe[2] = {-1, -1};
+	REQUIRE(pipe(ready_pipe) == 0);
+
+	const auto child_pid = fork();
+	REQUIRE(child_pid >= 0);
+
+	if (child_pid == 0) {
+		close(ready_pipe[0]);
+
+		std::vector<std::unique_ptr<KAutoSaveFile>> backups;
+		for (int i = 0; i < generations; ++i) {
+			auto backup = std::make_unique<KAutoSaveFile>(
+				QUrl::fromLocalFile(managed_path));
+			if (!backup->open(QIODevice::WriteOnly
+						  | QIODevice::Truncate
+						  | QIODevice::Text)) {
+				_exit(2);
+			}
+			const QByteArray payload =
+				"<project><generation n=\"" + QByteArray::number(i)
+				+ "\" /></project>\n";
+			if (backup->write(payload) != payload.size()) {
+				_exit(3);
+			}
+			if (!backup->flush()) {
+				_exit(4);
+			}
+				//Give each generation a distinct, increasing mtime.
+			struct timespec pause{0, 20 * 1000 * 1000};
+			nanosleep(&pause, nullptr);
+			backups.push_back(std::move(backup));
+		}
+
+		const char ready = '1';
+		if (write(ready_pipe[1], &ready, 1) != 1) {
+			_exit(5);
+		}
+		close(ready_pipe[1]);
+
+		for (;;) {
+			pause();
+		}
+	}
+
+	close(ready_pipe[1]);
+	char ready = 0;
+	REQUIRE(read(ready_pipe[0], &ready, 1) == 1);
+	close(ready_pipe[0]);
+	REQUIRE(ready == '1');
+
+	REQUIRE(kill(child_pid, SIGKILL) == 0);
+	int status = 0;
+	REQUIRE(waitpid(child_pid, &status, 0) == child_pid);
+	REQUIRE(WIFSIGNALED(status));
+	REQUIRE(WTERMSIG(status) == SIGKILL);
+
+	auto stale_files = KAutoSaveFile::allStaleFiles();
+	REQUIRE(stale_files.size() == generations);
+
+	std::sort(stale_files.begin(), stale_files.end(),
+		[](KAutoSaveFile *a, KAutoSaveFile *b) {
+			return QFileInfo(*a).lastModified() < QFileInfo(*b).lastModified();
+		});
+
+	for (int i = 0; i < generations; ++i) {
+		std::unique_ptr<KAutoSaveFile> stale_file(stale_files.at(i));
+		CHECK(stale_file->managedFile().path()
+			  == QFileInfo(managed_path).absoluteFilePath());
+		REQUIRE(stale_file->open(QIODevice::ReadOnly | QIODevice::Text));
+		const QByteArray content = stale_file->readAll();
+		CHECK(content.contains(
+			"generation n=\"" + QByteArray::number(i) + "\""));
+	}
+#endif
+}