[rkward/frameworks] rkward/rbackend: Some fixes for crashes / deadlocks related to priority commands:
Thomas Friedrichsmeier
thomas.friedrichsmeier at ruhr-uni-bochum.de
Mon Oct 24 10:51:37 UTC 2016
Git commit a6f12815a69f1532a84c8f72c7da920408977635 by Thomas Friedrichsmeier.
Committed on 24/10/2016 at 10:49.
Pushed by tfry into branch 'frameworks'.
Some fixes for crashes / deadlocks related to priority commands:
- On the frontend side, make sure to always attach sub-command stacks to the right command (not to whichever - possibly priority - command was last submitted)
- On the backend side, make sure not to lose track to previous command when processing a priority command while handling command out
M +1 -0 rkward/rbackend/rcommandstack.cpp
M +22 -9 rkward/rbackend/rinterface.cpp
M +1 -1 rkward/rbackend/rinterface.h
M +6 -2 rkward/rbackend/rkrbackend.cpp
M +1 -1 rkward/rbackend/rkrbackend.h
http://commits.kde.org/rkward/a6f12815a69f1532a84c8f72c7da920408977635
diff --git a/rkward/rbackend/rcommandstack.cpp b/rkward/rbackend/rcommandstack.cpp
index 0b7c64c..92bb3de 100644
--- a/rkward/rbackend/rcommandstack.cpp
+++ b/rkward/rbackend/rcommandstack.cpp
@@ -149,6 +149,7 @@ bool RCommandStack::popIfCompleted (RCommandChain* item) {
RK_TRACE (RBACKEND);
if (item->isClosed () && item->sub_commands.isEmpty () && item->parent && (!item->is_command)) { // if the item has no parent, it is the main stack. If it is a command, it will be popped from the RInterface.
+ RK_DEBUG (RBACKEND, DL_DEBUG, "popping completed chain: %p", item);
pop (item);
return true;
}
diff --git a/rkward/rbackend/rinterface.cpp b/rkward/rbackend/rinterface.cpp
index e4df74b..b058576 100644
--- a/rkward/rbackend/rinterface.cpp
+++ b/rkward/rbackend/rinterface.cpp
@@ -361,6 +361,11 @@ void RInterface::handleRequest (RBackendRequest* request) {
flushOutput (true);
if (request->type == RBackendRequest::CommandOut) {
RCommandProxy *cproxy = request->takeCommand ();
+ if (cproxy) {
+ RK_DEBUG (RBACKEND, DL_DEBUG, "Command out \"%s\", id %d", qPrintable (cproxy->command), cproxy->id);
+ } else {
+ RK_DEBUG (RBACKEND, DL_DEBUG, "Fake command out");
+ }
RCommand *command = 0;
// NOTE: the order of processing is: first try to submit the next command, then handle the old command.
// The reason for doing it this way, instead of the reverse, is that this allows the backend thread / process to continue working, concurrently
@@ -375,8 +380,16 @@ void RInterface::handleRequest (RBackendRequest* request) {
}
tryNextCommand ();
} else if (request->type == RBackendRequest::HistoricalSubstackRequest) {
+ RCommandProxy *cproxy = request->command;
+ RCommand *parent = 0;
+ for (int i = all_current_commands.size () - 1; i >= 0; --i) {
+ if (all_current_commands[i]->id () == cproxy->id) {
+ parent = all_current_commands[i];
+ break;
+ }
+ }
command_requests.append (request);
- processHistoricalSubstackRequest (request->params["call"].toStringList ());
+ processHistoricalSubstackRequest (request->params["call"].toStringList (), parent);
} else if (request->type == RBackendRequest::PlainGenericRequest) {
request->params["return"] = QVariant (processPlainGenericRequest (request->params["call"].toStringList ()));
RKRBackendProtocolFrontend::setRequestCompleted (request);
@@ -676,19 +689,19 @@ QStringList RInterface::processPlainGenericRequest (const QStringList &calllist)
return QStringList ();
}
-void RInterface::processHistoricalSubstackRequest (const QStringList &calllist) {
+void RInterface::processHistoricalSubstackRequest (const QStringList &calllist, RCommand *parent_command) {
RK_TRACE (RBACKEND);
- RCommand *current_command = runningCommand ();
RCommandChain *in_chain;
- if (!current_command) {
+ if (!parent_command) {
// This can happen for Tcl events. Create a dummy command on the stack to keep things looping.
- current_command = new RCommand (QString (), RCommand::App | RCommand::EmptyCommand | RCommand::Sync);
- RCommandStack::issueCommand (current_command, 0);
- all_current_commands.append (current_command);
- dummy_command_on_stack = current_command; // so we can get rid of it again, after it's sub-commands have finished
+ parent_command = new RCommand (QString (), RCommand::App | RCommand::EmptyCommand | RCommand::Sync);
+ RCommandStack::issueCommand (parent_command, 0);
+ all_current_commands.append (parent_command);
+ dummy_command_on_stack = parent_command; // so we can get rid of it again, after it's sub-commands have finished
}
- in_chain = openSubcommandChain (current_command);
+ in_chain = openSubcommandChain (parent_command);
+ RK_DEBUG (RBACKEND, DL_DEBUG, "started sub-command chain (%p) for command %s", in_chain, qPrintable (parent_command->command ()));
QString call = calllist.value (0);
if (call == "sync") {
diff --git a/rkward/rbackend/rinterface.h b/rkward/rbackend/rinterface.h
index d186f8f..086b399 100644
--- a/rkward/rbackend/rinterface.h
+++ b/rkward/rbackend/rinterface.h
@@ -92,7 +92,7 @@ private:
RecordingCommandsUnfiltered
} command_logfile_mode;
- void processHistoricalSubstackRequest (const QStringList &calllist);
+ void processHistoricalSubstackRequest (const QStringList &calllist, RCommand *parent_command);
QStringList processPlainGenericRequest (const QStringList &calllist);
void processRBackendRequest (RBackendRequest *request);
diff --git a/rkward/rbackend/rkrbackend.cpp b/rkward/rbackend/rkrbackend.cpp
index b4060ad..bc3eebb 100644
--- a/rkward/rbackend/rkrbackend.cpp
+++ b/rkward/rbackend/rkrbackend.cpp
@@ -1331,9 +1331,9 @@ void doPendingPriorityCommands () {
if (RKRBackend::this_pointer->killed) return;
RCommandProxy *command = RKRBackend::this_pointer->pending_priority_command;
+ RKRBackend::this_pointer->pending_priority_command = 0;
if (command) {
RK_DEBUG (RBACKEND, DL_DEBUG, "running priority command %s", qPrintable (command->command));
- RKRBackend::this_pointer->setPriorityCommand (0);
{
QMutexLocker lock (&RKRBackend::this_pointer->all_current_commands_mutex);
RKRBackend::this_pointer->all_current_commands.append (command);
@@ -1341,8 +1341,10 @@ void doPendingPriorityCommands () {
}
RKRBackend::this_pointer->runCommand (command);
- RKRBackend::this_pointer->commandFinished (false);
// TODO: Oh boy, what a mess. Sending notifications should be split from fetchNextCommand() (which is not appropriate, here)
+ RCommandProxy *previous_command_backup = RKRBackend::this_pointer->previous_command;
+ RKRBackend::this_pointer->commandFinished (false);
+ RKRBackend::this_pointer->previous_command = previous_command_backup;
RBackendRequest req (false, RBackendRequest::CommandOut); // NOTE: We do *NOT* want a reply to this one, and in particular, we do *NOT* want to do
// (recursive) event processing while handling this.
req.command = command;
@@ -1466,6 +1468,7 @@ RCommandProxy* RKRBackend::handleRequest (RBackendRequest *request, bool mayHand
if (!request->done) RKRBackendProtocolBackend::msleep (++i < 200 ? 10 : 50);
}
+ // TODO remove me?
while (pending_priority_command) RKREventLoop::processX11Events (); // Probably not needed, but make sure to process priority commands first at all times.
RCommandProxy* command = request->takeCommand ();
@@ -1513,6 +1516,7 @@ void RKRBackend::handleHistoricalSubstackRequest (const QStringList &list) {
RBackendRequest request (true, RBackendRequest::HistoricalSubstackRequest);
request.params["call"] = list;
+ request.command = current_command;
handleRequest (&request);
}
diff --git a/rkward/rbackend/rkrbackend.h b/rkward/rbackend/rkrbackend.h
index c933941..637db01 100644
--- a/rkward/rbackend/rkrbackend.h
+++ b/rkward/rbackend/rkrbackend.h
@@ -213,7 +213,7 @@ private:
QStringList global_env_toplevel_names;
/** check whether the object list / global environment / individual symbols have changed, and updates them, if needed */
void checkObjectUpdatesNeeded (bool check_list);
-
+friend void doPendingPriorityCommands ();
/** The previously executed command. Only non-zero until a new command has been requested. */
RCommandProxy *previous_command;
};
More information about the rkward-tracker
mailing list