Merge branch '94-commands-getting-lost' into 'develop'

Resolve "multiple extendedlist commands cause all following commands to fail"

Closes #94

See merge request tobias.wach/ccats!105

cli/include/cmdman.h

@@ -6,6 +6,7 @@
 #include <json/json.h>
 
 #include <map>
+#include <mutex>
 #include <string>
 #include <vector>
 
@@ -73,6 +74,12 @@ protected:
 	state currentState;
 
 private:
+	/**
+	 * Prevents multiple calls of execute and handle at the same time.
+	 * Thereby prevents an incorrect state of used cmdman and fileman instances.
+	 */
+	std::mutex cmdmutex;
+
 	/**
 	 * internal json writer and error string member
 	 */

cli/src/cmdman.cpp

@@ -414,6 +414,7 @@ CmdMan::CmdRet CmdMan::execute(string cmd, vector<string> args) {
 	for (string s : args)
 		DEBUGPRINT(s + " ");
 	DEBUGPRINT("]");
+	cmdmutex.lock();
 	map<string, CmdRet (CmdMan::*)(vector<string>)>::iterator execit = execmap.find(cmd);
 	vector<string>::const_iterator alwaysit;
 	vector<string>::const_iterator connectit;
@@ -431,6 +432,7 @@ CmdMan::CmdRet CmdMan::execute(string cmd, vector<string> args) {
 		root["command"] = "error";
 		root["error"] = string(__PRETTY_FUNCTION__) + " unknown command \"" + cmd + "\".\ntype help to list available commands.";
 		retval.msg = root;
+		cmdmutex.unlock();
 		return retval;
 	} else if (alwaysit != cmdAllowAlways.cend()) {
 		// Command should be usable in all cases
@@ -449,6 +451,7 @@ CmdMan::CmdRet CmdMan::execute(string cmd, vector<string> args) {
 			root["command"] = "error";
 			root["error"] = string("Not logged in. Available commands are limited to ") + allowedCommands + "\n" + "Use help for usage of these commands.";
 			retval.msg = root;
+			cmdmutex.unlock();
 			return retval;
 		}
 	} else if (currentState == versionpossible || currentState == doversion) {
@@ -459,6 +462,7 @@ CmdMan::CmdRet CmdMan::execute(string cmd, vector<string> args) {
 			root["command"] = "error";
 			root["error"] = string("Version not checked yet. No commands avalable.");
 			retval.msg = root;
+			cmdmutex.unlock();
 			return retval;
 		}
 	} else if (currentState == connectionpossible) {
@@ -469,10 +473,14 @@ CmdMan::CmdRet CmdMan::execute(string cmd, vector<string> args) {
 			root["command"] = "error";
 			root["error"] = string("Not connected. Connect using \"connect ip [port]\".");
 			retval.msg = root;
+			cmdmutex.unlock();
 			return retval;
 		}
 	}
-	return (this->*(execmap[cmd]))(args);
+
+	retval = (this->*(execmap[cmd]))(args);
+	cmdmutex.unlock();
+	return retval;
 }
 
 CmdMan::CmdRet CmdMan::cmdDeleteme(vector<string> args) {
@@ -696,6 +704,7 @@ CmdMan::CmdRet CmdMan::handle(Json::Value root) {
 	CmdRet retval;
 	Json::Value output;
 	DEBUGPRINT(string(__PRETTY_FUNCTION__) + " begin");
+	cmdmutex.lock();
 	if (currentState == doversion)
 		root["command"] = "version";
 	else if (currentState == dosignup)
@@ -711,9 +720,12 @@ CmdMan::CmdRet CmdMan::handle(Json::Value root) {
 		output["command"] = "error";
 		output["error"] = string(__PRETTY_FUNCTION__) + " unknown command \"" + root["command"].asString() + "\".\nEnsure code is implemented.";
 		retval.msg = output;
+		cmdmutex.unlock();
 		return retval;
 	}
-	return (this->*(handlemap[root["command"].asString()]))(root);
+	retval = (this->*(handlemap[root["command"].asString()]))(root);
+	cmdmutex.unlock();
+	return retval;
 }
 
 CmdMan::CmdRet CmdMan::handleStatus(Json::Value root) {

cli/src/ioman.cpp

@@ -232,6 +232,8 @@ void IoMan::networkMain() {
 	Json::Value root;
 	unsigned int jsonsize, readsize;
 
+	bool firstWasGood = false;
+
 	printMessage("IoMan::networkMain() begin", debug);
 	networkmutex.lock();
 	while (runnetwork) {
@@ -263,7 +265,7 @@ void IoMan::networkMain() {
 			continue;
 		}
 		recvjson = (char *)(boost::asio::buffer_cast<const char *>(recvbuf.data()));
-		recvjson[readsize] = 0;
+		recvjson[recvbuf.size()] = 0;
 		while (strchr(recvjson, '\n')) {
 			// parse
 			jsonsize = strchr(recvjson, '\n') - recvjson + 1;
@@ -272,12 +274,17 @@ void IoMan::networkMain() {
 
 			if (!reader->parse(recvjson, recvjson + jsonsize, &root, &jsonerror)) {
 				printMessage("IoMan::networkMain() couldnt parse json data: " + jsonerror, debug);
+				if (firstWasGood) {
+					// we found garbage at the end
+					break;
+				}
+				// we found garbage at the beginning
 				recvbuf.consume(jsonsize);
 				recvjson += jsonsize;
 				continue;
 			}
+			firstWasGood = true;
 			recvbuf.consume(jsonsize);
-			readsize -= jsonsize;
 
 			printMessage(string(__PRETTY_FUNCTION__) + string(" remaining recvbuf ") + string(boost::asio::buffer_cast<const char *>(recvbuf.data())), debug);
 
@@ -285,6 +292,7 @@ void IoMan::networkMain() {
 			// store locally
 			toput.push_back(root);
 		}
+		firstWasGood = false;
 
 		if (toput.size()) {
 			// put into global vector
@@ -298,7 +306,7 @@ void IoMan::networkMain() {
 
 		// clean up local stuff
 		toput = vector<Json::Value>();
-		recvbuf.consume(readsize);
+		recvbuf.consume(recvbuf.size() + 1);
 		networkmutex.lock();
 	}
 }

gui/src/climanager.cpp

@@ -73,7 +73,7 @@ void CliManager::init() {
 		// Child
 		dup2(outpipefd[0], STDIN_FILENO);
 		dup2(inpipefd[1], STDOUT_FILENO);
-		// dup2(inpipefd[1], STDERR_FILENO);
+		 dup2(inpipefd[1], STDERR_FILENO);
 
 		// ask kernel to deliver SIGTERM in case the parent dies
 		prctl(PR_SET_PDEATHSIG, SIGTERM);