[education/rkward/releases/0.8.1] rkward: Fix corner case in interrupting user commands

[Thomas Friedrichsmeier]

Git commit f7a220b2998f0b4e1e58ad3e69a435cb953f3778 by Thomas Friedrichsmeier.
Committed on 18/04/2025 at 12:39.
Pushed by tfry into branch 'releases/0.8.1'.

Fix corner case in interrupting user commands

M  +14   -9    rkward/autotests/core_test.cpp
M  +2    -1    rkward/rbackend/rkrbackend.cpp

https://invent.kde.org/education/rkward/-/commit/f7a220b2998f0b4e1e58ad3e69a435cb953f3778

diff --git a/rkward/autotests/core_test.cpp b/rkward/autotests/core_test.cpp
index e894ae55e..b728dd572 100644
--- a/rkward/autotests/core_test.cpp
+++ b/rkward/autotests/core_test.cpp
@@ -70,17 +70,19 @@ class RKWardCoreTest: public QObject {
 	Q_OBJECT
 
 	void runCommandWithTimeout(RCommand *command, RCommandChain* chain, std::function<void(RCommand*)> callback, int timeoutms = 1000) {
+		static int done;
 		QString ccopy = command->command();
+		auto command_id = command->id();
 		QElapsedTimer t;
 		t.start();
-		bool done = false;
-		bool *_done = &done;
-		connect(command->notifier(), &RCommandNotifier::commandFinished, this, [_done, callback](RCommand *command) { *_done = true; callback(command); });
+		done = -1;
+		int *_done = &done;
+		connect(command->notifier(), &RCommandNotifier::commandFinished, this, [_done, command_id, callback](RCommand *command) { *_done = command_id; callback(command); });
 		RInterface::issueCommand(command, chain);
-		while (!done && (t.elapsed() < timeoutms)) {
+		while ((done != command_id) && (t.elapsed() < timeoutms)) {
 			qApp->processEvents(QEventLoop::AllEvents, 500);
 		}
-		if (!done) {
+		if (done != command_id) {
 			testLog("Command timed out: %s", qPrintable(ccopy));
 			QFAIL("Command timed out");
 		}
@@ -402,18 +404,21 @@ private Q_SLOTS:
 
 	void priorityCommandTest() {
 		bool priority_command_done = false;
-		runCommandAsync(new RCommand("cat(\"sleeping\n\"); Sys.sleep(5)", RCommand::User), nullptr, [&priority_command_done](RCommand *command) {
+		runCommandAsync(new RCommand("\ncat(\"sleeping\\n\"); 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);
+		auto priority_command = new RCommand("cat(\"priority\\n\")", RCommand::PriorityCommand | RCommand::App);
 		runCommandAsync(priority_command, nullptr, [&priority_command_done](RCommand *) {
 			priority_command_done = true;
 			RInterface::instance()->cancelAll();
 		});
-		waitForAllFinished();
-		waitForAllFinished(5000);  // priority_command_done must remain in scope until done (even if interrupting fails for some reason)
+		// NOTE: The above two commands may or may not run in that order. Conceivably, the priority command gets handled, before the initial sleep command even started.
+		//       The newline in the first command actually makes it a bit more likely for the priority command to go first (because parsing needs more iterations).
+		//       We try to step on that interesting corner case, deliberately, at is has been causing failures in the past.
+		waitForAllFinished();      // first wait with a short timeout: sleep should have been cancelled
+		waitForAllFinished(5000);  // fallbacck: priority_command_done must remain in scope until done (even if interrupting fails for some reason)
 	}
 
 	void RKConsoleHistoryTest() {
diff --git a/rkward/rbackend/rkrbackend.cpp b/rkward/rbackend/rkrbackend.cpp
index 6e94a1925..a884fdaf0 100644
--- a/rkward/rbackend/rkrbackend.cpp
+++ b/rkward/rbackend/rkrbackend.cpp
@@ -164,9 +164,11 @@ void RKTransmitNextUserCommandChunk (unsigned char* buf, int buflen) {
 		RKRBackend::repl_status.user_command_completely_transmitted = true;
 	}
 	buf[++pos] = '\0';
+	RKRBackend::repl_status.user_command_status = RKRBackend::RKReplStatus::UserCommandTransmitted;
 
 	if (reached_newline || reached_eof) {
 		// Making this request synchronous is a bit painful. However, without this, it's extremely difficult to get correct interleaving of output and command lines
+		RKRSupport::InterruptSuspension susp; // This could also result in interrupts, in corner cases, so lets suspend those, for the minute
 		RBackendRequest req (true, RBackendRequest::CommandLineIn);
 		req.params["commandid"] = RKRBackend::this_pointer->current_command->id;
 		RKRBackend::this_pointer->handleRequest (&req);
@@ -224,7 +226,6 @@ int RReadConsole (const char* prompt, unsigned char* buf, int buflen, int hist)
 					RKRBackend::repl_status.user_command_successful_up_to = 0;
 					RKRBackend::repl_status.user_command_buffer = RKTextCodec::toNative(command->command);
 					RKTransmitNextUserCommandChunk (buf, buflen);
-					RKRBackend::repl_status.user_command_status = RKRBackend::RKReplStatus::UserCommandTransmitted;
 					return 1;
 				}
 			} else if (RKRBackend::repl_status.user_command_status == RKRBackend::RKReplStatus::UserCommandTransmitted) {