[education/rkward] rkward: Cleanups around backend restart feature

Thomas Friedrichsmeier null at kde.org
Sun Jun 5 08:52:41 BST 2022


Git commit 967bf17b4840a54648457cd0a6c7dd9e0231f30a by Thomas Friedrichsmeier.
Committed on 05/06/2022 at 07:52.
Pushed by tfry into branch 'master'.

Cleanups around backend restart feature

M  +3    -1    rkward/rbackend/rkrbackendprotocol_frontend.cpp
M  +2    -0    rkward/rbackend/rkrinterface.cpp
M  +9    -7    rkward/rbackend/rktransmitter.cpp
M  +30   -7    rkward/rkward.cpp

https://invent.kde.org/education/rkward/commit/967bf17b4840a54648457cd0a6c7dd9e0231f30a

diff --git a/rkward/rbackend/rkrbackendprotocol_frontend.cpp b/rkward/rbackend/rkrbackendprotocol_frontend.cpp
index 0a1b2412..fd09de28 100644
--- a/rkward/rbackend/rkrbackendprotocol_frontend.cpp
+++ b/rkward/rbackend/rkrbackendprotocol_frontend.cpp
@@ -1,6 +1,6 @@
 /*
 rkrbackendprotocol - This file is part of RKWard (https://rkward.kde.org). Created: Thu Nov 04 2010
-SPDX-FileCopyrightText: 2010-2011 by Thomas Friedrichsmeier <thomas.friedrichsmeier at kdemail.net>
+SPDX-FileCopyrightText: 2010-2022 by Thomas Friedrichsmeier <thomas.friedrichsmeier at kdemail.net>
 SPDX-FileContributor: The RKWard Team <rkward-devel at kde.org>
 SPDX-License-Identifier: GPL-2.0-or-later
 */
@@ -28,12 +28,14 @@ RKRBackendProtocolFrontend::RKRBackendProtocolFrontend (RInterface* parent) : QO
 RKRBackendProtocolFrontend::~RKRBackendProtocolFrontend () {
 	RK_TRACE (RBACKEND);
 
+	RK_ASSERT(_instance == this);
 	terminateBackend ();
 	RKFrontendTransmitter::instance ()->wait(1000);  // Wait for thread to catch the backend's exit request, and exit()
 	RKFrontendTransmitter::instance ()->quit();      // Tell it to quit, otherwise
 	RKFrontendTransmitter::instance ()->wait(3000);  // Wait for thread to quit and clean up.
 	qApp->processEvents(QEventLoop::AllEvents, 500); // Not strictly needed, but avoids some mem leaks on exit by handling all posted BackendExit events
 	delete RKFrontendTransmitter::instance ();
+	_instance = nullptr;
 }
 
 void RKRBackendProtocolFrontend::setupBackend () {
diff --git a/rkward/rbackend/rkrinterface.cpp b/rkward/rbackend/rkrinterface.cpp
index 9d53fc89..ea33f897 100644
--- a/rkward/rbackend/rkrinterface.cpp
+++ b/rkward/rbackend/rkrinterface.cpp
@@ -113,7 +113,9 @@ RInterface::~RInterface(){
 
 	// Don't wait for QObject d'tor to destroy the backend transmitter. It might still try to call functions in the RInterface
 	// (noteably, it does call qApp->processEvents().
+	RK_ASSERT(_instance == this);
 	delete RKRBackendProtocolFrontend::instance ();
+	_instance = nullptr;
 	RKWindowCatcher::discardInstance ();
 }
 
diff --git a/rkward/rbackend/rktransmitter.cpp b/rkward/rbackend/rktransmitter.cpp
index 0d9c2960..2ace9416 100644
--- a/rkward/rbackend/rktransmitter.cpp
+++ b/rkward/rbackend/rktransmitter.cpp
@@ -192,19 +192,21 @@ RCommandProxy* RKRBackendSerializer::unserializeProxy (QDataStream &stream) {
 
 #include <QTimer>
 #include <QLocalSocket>
-RKAbstractTransmitter* RKAbstractTransmitter::_instance = 0;
-RKAbstractTransmitter::RKAbstractTransmitter () : QThread () {
+RKAbstractTransmitter* RKAbstractTransmitter::_instance = nullptr;
+RKAbstractTransmitter::RKAbstractTransmitter() : QThread() {
 	RK_TRACE (RBACKEND);
 
-	RK_ASSERT (_instance == 0);	// NOTE: Although there are two instances of an abstract transmitter in an RKWard session, these live in different processes.
+	RK_ASSERT(_instance == nullptr);  // NOTE: Although there are two instances of an abstract transmitter in an RKWard session, these live in different processes.
 	_instance = this;
-	connection = 0;
+	connection = nullptr;
 
-	moveToThread (this);
+	moveToThread(this);
 }
 
-RKAbstractTransmitter::~RKAbstractTransmitter () {
-	RK_TRACE (RBACKEND);
+RKAbstractTransmitter::~RKAbstractTransmitter() {
+	RK_TRACE(RBACKEND);
+	RK_ASSERT(_instance == this);
+	_instance = nullptr;
 }
 
 void RKAbstractTransmitter::transmitRequest (RBackendRequest *request) {
diff --git a/rkward/rkward.cpp b/rkward/rkward.cpp
index c9f70587..d0bb641c 100644
--- a/rkward/rkward.cpp
+++ b/rkward/rkward.cpp
@@ -615,15 +615,38 @@ void RKWardMainWindow::initActions() {
 	auto restart_r = actionCollection()->addAction("restart_r");
 	restart_r->setText(i18n("Restart R Backend"));
 	connect(restart_r, &QAction::triggered, this, [this]() {
-		bool pending = !RInterface::instance()->backendIsDead() && !RInterface::instance()->backendIsIdle();
-		QString add = pending ? i18n("<p>One or more operations are pending, and will be canceled. If you have recently chosen to save your workspace, and you see this message, <b>your data may not be saved, yet!</b></p>") : QString();
-		QString message = i18n("<p>This feature is primarily targetted at package developers, who know what they are doing. Please proceed with caution.</p><p><b>All unsaved data in this workspace will be lost!</b> All data editors, and graphics windows will be closed.</p>%1<p>Are you sure you want to proceed?</p>", add);
-		if (KMessageBox::warningContinueCancel(this, message, i18n("Restart R backend"), KGuiItem("Restart R backend now"), KGuiItem("Cancel")) == KMessageBox::Continue) {
+		QString message = i18n("<p>This feature is primarily targetted at package developers, who know what they are doing. Please proceed with caution.</p><p><b>All unsaved data in this workspace will be lost!</b> All data editors, and graphics windows will be closed.</p><p>Are you sure you want to proceed?</p>");
+		if (KMessageBox::warningContinueCancel(this, message, i18n("Restart R backend"), KGuiItem(i18n("Restart R backend"))) == KMessageBox::Continue) {
+			bool forced = RInterface::instance()->backendIsDead();
+			while (!RInterface::instance()->backendIsDead() && !RInterface::instance()->backendIsIdle()) {
+				message = i18n("<p>One or more operations are pending.</p><p>If you have recently chosen to save your workspace, and you see this message, <b>your data may not be saved, yet!</b></p><p>How do you want to proceed?</p>");
+				auto res = KMessageBox::warningYesNoCancel(this, message, i18n("R commands still pending"), KGuiItem(i18n("Force restart now")), KGuiItem(i18n("Check again")), KGuiItem(i18n("Cancel restarting")));
+				if (res == KMessageBox::Yes) {
+					forced = true;
+					break;
+				} else if (res == KMessageBox::Cancel) {
+					return;
+				}
+				// KMessageBox::No means to loop: Commands may have finished, meanwhile. This is not really pretty, a proper progress control would be nicer,
+				// but then, this is a corner case to a feature that is not really targetted at a mainstream audience, anyway.
+			}
+
 			RKWorkplace::mainWorkplace()->closeAll(RKMDIWindow::X11Window);
 			slotCloseAllEditors();
-			delete RInterface::instance();
-			RKWorkplace::mainWorkplace()->setWorkspaceURL(QUrl());
-			startR();
+			auto restart_now = [this]() {
+				delete RInterface::instance();  // NOTE: Do not use deleteLater(), here. It is important to fully tear down the old backend, before creating the new one,
+				                                //       as code is written around the assumption that RInterface and friends are singletons. (RInterface::instance(), etc.)
+				RKWorkplace::mainWorkplace()->setWorkspaceURL(QUrl());
+				startR();
+			};
+			if (forced) {
+				RKConsole::mainConsole()->resetConsole();
+				restart_now();
+			} else {
+				RCommand *c = new RCommand(QString(), RCommand::QuitCommand);
+				c->whenFinished(this, [restart_now]() { QTimer::singleShot(0, restart_now); });
+				RInterface::issueCommand(c);
+			}
 		}
 	});
 }


More information about the rkward-tracker mailing list