[education/rkward] /: Fix backend restart on windows
Thomas Friedrichsmeier
null at kde.org
Sun Oct 2 22:26:34 BST 2022
Git commit 7386947187c720e6cb06a73e0df9dc11d3144dbc by Thomas Friedrichsmeier.
Committed on 02/10/2022 at 21:26.
Pushed by tfry into branch 'master'.
Fix backend restart on windows
Backend restart on Windows would hang under unknown conditions (frequently, on the CI, however).
M +2 -2 .kde-ci.yml
M +1 -0 ChangeLog
M +38 -37 rkward/autotests/core_test.cpp
M +4 -3 rkward/rbackend/rkfrontendtransmitter.cpp
M +1 -1 rkward/rbackend/rkrbackendprotocol_frontend.cpp
M +2 -0 rkward/rkward.cpp
M +1 -1 rkward/windows/rkwindowcatcher.h
https://invent.kde.org/education/rkward/commit/7386947187c720e6cb06a73e0df9dc11d3144dbc
diff --git a/.kde-ci.yml b/.kde-ci.yml
index 0461eeb4..5e58bc28 100644
--- a/.kde-ci.yml
+++ b/.kde-ci.yml
@@ -19,6 +19,6 @@ Dependencies:
'frameworks/kcrash': '@stable'
Options:
- require-passing-tests-on: [ 'Linux', 'FreeBSD' ]
+ require-passing-tests-on: [ 'Linux', 'FreeBSD', 'Windows' ]
tests-load-sensitve: True
- per-test-timeout: 120
+ per-test-timeout: 60
diff --git a/ChangeLog b/ChangeLog
index e1e29e54..273f7cb2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,4 @@
+- Fixed: Trying to restart backend could cause a hang, on Windows
- Fixed: In corner cases, cancelling commands could lead to a lockup
- Fixed: IRT Cronbach's Alpha did not work for subsets, if the data.frame name contains dots
- Fixed: Action to remove several rows in data editor, simultaneously, always remained disabled
diff --git a/rkward/autotests/core_test.cpp b/rkward/autotests/core_test.cpp
index 2857629d..763f69e0 100644
--- a/rkward/autotests/core_test.cpp
+++ b/rkward/autotests/core_test.cpp
@@ -33,6 +33,7 @@ void testLog(const char* fmt, va_list args) {
printf("%lld: ", _test_timer.elapsed());
vprintf(fmt, args);
printf("\n");
+ fflush(stdout);
}
void testLog(const char* fmt, ...) {
@@ -209,43 +210,6 @@ private slots:
cleanGlobalenv();
}
- void restartRBackend() {
- auto restart_action = RKWardMainWindow::getMain()->actionCollection()->action("restart_r");
- QVERIFY(restart_action != nullptr);
- RInterface::issueCommand(new RCommand("x <- 1", RCommand::User));
- waitForAllFinished();
- QVERIFY(RObjectList::getGlobalEnv()->findObject("x"));
-
- QPointer<RInterface> oldiface = RInterface::instance();
- restart_action->trigger();
- while (oldiface) { // action may be delayed until next event processing
- qApp->processEvents();
- }
- waitForBackendStarted();
-
- // backend should be clean after restart
- QVERIFY(!RObjectList::getGlobalEnv()->findObject("x"));
- // but of course it should also be functional...
- RInterface::issueCommand(new RCommand("x <- 1", RCommand::User));
- waitForAllFinished();
- QVERIFY(RObjectList::getGlobalEnv()->findObject("x"));
- }
-
- void priorityCommandTest() {
- bool priority_command_done = false;
- runCommandAsync(new RCommand("Sys.sleep(5)", RCommand::User), nullptr, [&priority_command_done](RCommand *command) {
- QVERIFY(priority_command_done);
- QVERIFY(command->failed());
- QVERIFY(command->wasCanceled());
- });
- auto priority_command = new RCommand("cat(\"something\\n\")", RCommand::PriorityCommand | RCommand::App);
- runCommandAsync(priority_command, nullptr, [&priority_command_done](RCommand *) {
- priority_command_done = true;
- RInterface::instance()->cancelAll();
- });
- waitForAllFinished(); // priority_command_done must remain in scope until done
- }
-
void commandOrderAndOutputTest() {
// commands shall run in the order 1, 3, 2, 5, 4, but also, of course, all different types of output shall be captured
QStringList output;
@@ -303,6 +267,43 @@ private slots:
testLog("%d out of %d commands were actually cancelled", cancelled_commands, commands_out);
}
+ void priorityCommandTest() {
+ bool priority_command_done = false;
+ runCommandAsync(new RCommand("Sys.sleep(5)", RCommand::User), nullptr, [&priority_command_done](RCommand *command) {
+ QVERIFY(priority_command_done);
+ QVERIFY(command->failed());
+ QVERIFY(command->wasCanceled());
+ });
+ auto priority_command = new RCommand("cat(\"something\\n\")", RCommand::PriorityCommand | RCommand::App);
+ runCommandAsync(priority_command, nullptr, [&priority_command_done](RCommand *) {
+ priority_command_done = true;
+ RInterface::instance()->cancelAll();
+ });
+ waitForAllFinished(); // priority_command_done must remain in scope until done
+ }
+
+ void restartRBackend() {
+ auto restart_action = RKWardMainWindow::getMain()->actionCollection()->action("restart_r");
+ QVERIFY(restart_action != nullptr);
+ RInterface::issueCommand(new RCommand("x <- 1", RCommand::User));
+ waitForAllFinished();
+ QVERIFY(RObjectList::getGlobalEnv()->findObject("x"));
+
+ QPointer<RInterface> oldiface = RInterface::instance();
+ restart_action->trigger();
+ while (oldiface) { // action may be delayed until next event processing
+ qApp->processEvents();
+ }
+ waitForBackendStarted();
+
+ // backend should be clean after restart
+ QVERIFY(!RObjectList::getGlobalEnv()->findObject("x"));
+ // but of course it should also be functional...
+ RInterface::issueCommand(new RCommand("x <- 1", RCommand::User));
+ waitForAllFinished();
+ QVERIFY(RObjectList::getGlobalEnv()->findObject("x"));
+ }
+
void cleanupTestCase()
{
// at least the backend should exit properly, to avoid creating emergency save files
diff --git a/rkward/rbackend/rkfrontendtransmitter.cpp b/rkward/rbackend/rkfrontendtransmitter.cpp
index 7ca0f178..db49c694 100644
--- a/rkward/rbackend/rkfrontendtransmitter.cpp
+++ b/rkward/rbackend/rkfrontendtransmitter.cpp
@@ -53,7 +53,6 @@ RKFrontendTransmitter::~RKFrontendTransmitter () {
RK_TRACE (RBACKEND);
delete rkd_transmitter;
- RK_ASSERT (!server->isListening ());
}
QString localeDir () {
@@ -171,13 +170,15 @@ void RKFrontendTransmitter::run () {
if (!quirkmode) {
// It's ok to only give backend a short time to finish. We only get here, after QuitCommand has been handled by the backend
- backend->waitForFinished(1000);
+ if (!backend->waitForFinished(1000)) backend->close();
}
if (!connection) {
RK_ASSERT (false);
- return;
}
+ RK_ASSERT(!server->isListening ());
+ delete server;
+ delete backend;
}
QString RKFrontendTransmitter::waitReadLine (QIODevice* con, int msecs) {
diff --git a/rkward/rbackend/rkrbackendprotocol_frontend.cpp b/rkward/rbackend/rkrbackendprotocol_frontend.cpp
index fd09de28..f9630f0f 100644
--- a/rkward/rbackend/rkrbackendprotocol_frontend.cpp
+++ b/rkward/rbackend/rkrbackendprotocol_frontend.cpp
@@ -31,7 +31,7 @@ RKRBackendProtocolFrontend::~RKRBackendProtocolFrontend () {
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
+ QMetaObject::invokeMethod(RKFrontendTransmitter::instance(), &RKFrontendTransmitter::quit, Qt::QueuedConnection); // 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 ();
diff --git a/rkward/rkward.cpp b/rkward/rkward.cpp
index 9672a7f5..db59c030 100644
--- a/rkward/rkward.cpp
+++ b/rkward/rkward.cpp
@@ -632,6 +632,7 @@ void RKWardMainWindow::initActions() {
if (suppressModalDialogsForTesting() || (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()) {
+ RK_DEBUG(APP, DL_DEBUG, "Backend not idle while restart requested.");
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) {
@@ -647,6 +648,7 @@ void RKWardMainWindow::initActions() {
RKWorkplace::mainWorkplace()->closeAll(RKMDIWindow::X11Window);
slotCloseAllEditors();
auto restart_now = [this]() {
+ RK_DEBUG(APP, DL_DEBUG, "Backend restart now");
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());
diff --git a/rkward/windows/rkwindowcatcher.h b/rkward/windows/rkwindowcatcher.h
index 6284ccf3..2cc9dc4b 100644
--- a/rkward/windows/rkwindowcatcher.h
+++ b/rkward/windows/rkwindowcatcher.h
@@ -76,7 +76,7 @@ public:
/** remove a watch created with registerNameWatcher */
void unregisterWatcher (WId watched);
static RKWindowCatcher *instance ();
- static void discardInstance () { delete _instance; };
+ static void discardInstance () { delete _instance; _instance = nullptr; };
private:
void pollWatchedWindowStates ();
QTimer poll_timer;
More information about the rkward-tracker
mailing list