From add28d0a7f265f5fcfdd67a4c1e590b19f76272e Mon Sep 17 00:00:00 2001
From: Sam Moore <matches@ucc.asn.au>
Date: Sat, 5 May 2012 21:03:33 +0800
Subject: [PATCH] [PATCH] Handle case where an AI sends an invalid message and
 then crashes

If an AI sent a bad move and then crashed, a SIGPIPE would be reported
rather than the AI's bad move.

NOTE: There is an issue with Program::Running, because the segfaulted AI
still returns 'true', which leads to confusion in handling SIGPIPE.

Should probably handle segfaulting AIs better.
Then again, people should probably not make segfaulting AIs.

[SZM]
---
 judge/manager/ai_controller.cpp    |   3 +-
 judge/manager/ai_controller.h      |   4 +-
 judge/manager/controller.cpp       |   6 +-
 judge/manager/controller.h         |   4 +-
 judge/manager/game.cpp             | 155 ++++++++++++++++++++---------
 judge/manager/game.h               |   3 +-
 judge/manager/human_controller.h   |   2 +-
 judge/manager/main.cpp             |  59 +----------
 judge/manager/network_controller.h |   6 +-
 judge/manager/program.cpp          |   1 +
 10 files changed, 129 insertions(+), 114 deletions(-)

diff --git a/judge/manager/ai_controller.cpp b/judge/manager/ai_controller.cpp
index 40ee3df..b73520f 100644
--- a/judge/manager/ai_controller.cpp
+++ b/judge/manager/ai_controller.cpp
@@ -54,11 +54,12 @@ MovementResult AI_Controller::QueryMove(string & buffer)
 	if (!Running())
 		return MovementResult::NO_MOVE; //AI has quit
 	Game::theGame->theBoard.Print(output, colour);
-
+	//Game::theGame->logMessage("DEBUG: About to get message from %d\n", colour);
 	if (!GetMessage(buffer,timeout))
 	{
 		return MovementResult::NO_MOVE; //AI did not respond (within the timeout). It will lose by default.
 	}
+	//Game::theGame->logMessage("DEBUG: Got message \"%s\" from %d\n", buffer.c_str(), colour);
 	return MovementResult::OK; //Got the message
 }
 
diff --git a/judge/manager/ai_controller.h b/judge/manager/ai_controller.h
index 0a4baeb..a5c791d 100644
--- a/judge/manager/ai_controller.h
+++ b/judge/manager/ai_controller.h
@@ -19,10 +19,10 @@ class AI_Controller : public Controller, private Program
 		virtual MovementResult QuerySetup(const char * opponentName,std::string setup[]);
 		virtual MovementResult QueryMove(std::string & buffer);
 
-		virtual void Message(const char * message) 
+		virtual bool Message(const char * message) 
 		{
 			//fprintf(stderr, "Sending message \"%s\" to AI program...\n", message);
-			Program::SendMessage(message);
+			return Program::SendMessage(message);
 		}
 		virtual void Pause() {Program::Pause();} //Hack wrapper
 		virtual void Continue() {Program::Continue();} //Hack wrapper
diff --git a/judge/manager/controller.cpp b/judge/manager/controller.cpp
index 20b79aa..e31c1ee 100644
--- a/judge/manager/controller.cpp
+++ b/judge/manager/controller.cpp
@@ -104,7 +104,7 @@ MovementResult Controller::MakeMove(string & buffer)
 	}
 
 
-	
+
 	int x; int y; string direction="";
 	stringstream s(buffer);
 	s >> x;
@@ -112,6 +112,9 @@ MovementResult Controller::MakeMove(string & buffer)
 	
 
 	s >> direction;
+
+
+
 	Board::Direction dir;
 	if (direction == "UP")
 	{
@@ -131,6 +134,7 @@ MovementResult Controller::MakeMove(string & buffer)
 	}	
 	else
 	{
+
 		if (Game::theGame->allowIllegalMoves)
 			return MovementResult::OK;
 		else
diff --git a/judge/manager/controller.h b/judge/manager/controller.h
index 069ccd6..361f309 100644
--- a/judge/manager/controller.h
+++ b/judge/manager/controller.h
@@ -21,8 +21,8 @@ class Controller
 
 		virtual bool HumanController() const {return false;} //Hacky... overrides in human_controller... avoids having to use run time type info
 
-		void Message(const std::string & buffer) {Message(buffer.c_str());}
-		virtual void Message(const char * string) = 0;
+		bool Message(const std::string & buffer) {return Message(buffer.c_str());}
+		virtual bool Message(const char * string) = 0;
 
 		virtual MovementResult QuerySetup(const char * opponentName, std::string setup[]) = 0;
 		virtual MovementResult QueryMove(std::string & buffer) = 0;
diff --git a/judge/manager/game.cpp b/judge/manager/game.cpp
index f50ce27..3f5b46b 100644
--- a/judge/manager/game.cpp
+++ b/judge/manager/game.cpp
@@ -264,62 +264,61 @@ void Game::HandleBrokenPipe(int sig)
 		fprintf(stderr, "ERROR - Recieved SIGPIPE during game exit!\n");
 		exit(EXIT_FAILURE);
 	}
-	if (theGame->turn == Piece::RED)
-	{
-		theGame->logMessage("Game ends on RED's turn - REASON: ");
-		if (theGame->blue->Valid()) //Should probably check this
-			theGame->blue->Message("DEFAULT");	
-	}
-	else if (theGame->turn == Piece::BLUE)
-	{
-	
-		theGame->logMessage("Game ends on BLUE's turn - REASON: ");
-		if (theGame->red->Valid()) //Should probably check this
-			theGame->red->Message("DEFAULT");
-	}
-	else
-	{
-		theGame->logMessage("Game ends on ERROR's turn - REASON: ");
-			
-	}
 	
 	theGame->logMessage("SIGPIPE - Broken pipe (AI program no longer running)\n");
 
-	if (Game::theGame->printBoard)
-		Game::theGame->theBoard.PrintPretty(stdout, Piece::BOTH);
+	MovementResult result = MovementResult::BAD_RESPONSE; 
 
-	
-	#ifdef BUILD_GRAPHICS
-	if (Game::theGame->graphicsEnabled && theGame->log == stdout)
+	if (theGame->turn == Piece::RED)
 	{
-		theGame->logMessage("CLOSE WINDOW TO EXIT\n");
-		Game::theGame->theBoard.Draw(Piece::BOTH);
-		while (true)
+		if (theGame->red->Valid())
 		{
-			SDL_Event  event;
-			while (SDL_PollEvent(&event))
+			theGame->logMessage("	Strange; RED still appears valid.\n");
+			if (theGame->blue->Valid())
 			{
-				switch (event.type)
-				{
-					case SDL_QUIT:
-						exit(EXIT_SUCCESS);
-						break;
-				}
-			}			
+				theGame->logMessage("	BLUE also appears valid. Exiting with ERROR.\n");
+				result = MovementResult::ERROR;
+			}
+			else
+			{
+				theGame->logMessage("BLUE is invalid. Wait for BLUE's turn to exit.\n");
+				return;
+			}
 		}
 	}
-	else
-	#endif //BUILD_GRAPHICS
+	if (theGame->turn == Piece::BLUE)
 	{
-		if (theGame->log == stdout || theGame->log == stderr)
+		if (theGame->blue->Valid())
 		{
-			theGame->logMessage( "PRESS ENTER TO EXIT\n");
-			theGame->theBoard.Print(theGame->log);
-			while (fgetc(stdin) != '\n');
+			theGame->logMessage("	Strange; BLUE still appears valid.\n");
+			if (theGame->red->Valid())
+			{
+				theGame->logMessage("	RED also appears valid. Exiting with ERROR.\n");
+				result = MovementResult::ERROR;
+			}
+			else
+			{
+				theGame->logMessage("RED is invalid. Wait for RED's turn to exit.\n");
+				return;
+			}
 		}
 	}
-	
 
+
+	Game::theGame->PrintEndMessage(result);
+
+	string buffer = "";
+	PrintResults(result, buffer);
+
+	//Message the AI's the quit message
+	Game::theGame->red->Message("QUIT " + buffer);
+	Game::theGame->blue->Message("QUIT " + buffer);
+
+	//Log the message
+	if (Game::theGame->GetLogFile() != stdout)
+		Game::theGame->logMessage("%s\n", buffer.c_str());
+
+	fprintf(stdout, "%s\n", buffer.c_str());
 	exit(EXIT_SUCCESS);
 }
 
@@ -425,6 +424,7 @@ void Game::PrintEndMessage(const MovementResult & result)
 			}
 			break;
 
+
 	}
 
 	if (printBoard)
@@ -541,8 +541,6 @@ MovementResult Game::Play()
 
 		logMessage( "%d RED: ", turnCount);
 		result = red->MakeMove(buffer);
-		red->Message(buffer);
-		blue->Message(buffer);
 		logMessage( "%s\n", buffer.c_str());
 
 		if (!Board::HaltResult(result))
@@ -552,6 +550,12 @@ MovementResult Game::Play()
 		if (Board::HaltResult(result))
 			break;
 
+		red->Message(buffer);
+		blue->Message(buffer);
+
+
+
+
 		if (stallTime >= 0)
 			Wait(stallTime);
 		else
@@ -595,8 +599,6 @@ MovementResult Game::Play()
 
 		logMessage( "%d BLU: ", turnCount);
 		result = blue->MakeMove(buffer);
-		blue->Message(buffer);
-		red->Message(buffer);
 		logMessage( "%s\n", buffer.c_str());
 
 		if (!Board::HaltResult(result))
@@ -606,6 +608,10 @@ MovementResult Game::Play()
 		if (Board::HaltResult(result))
 			break;
 
+		red->Message(buffer);
+		blue->Message(buffer);
+
+
 		if (theBoard.MobilePieces(Piece::RED) == 0)
 			result = MovementResult::DRAW;
 
@@ -904,3 +910,60 @@ string itostr(int i)
 	return s.str();
 }
 
+
+void Game::PrintResults(const MovementResult & result, string & buffer)
+{
+	stringstream s("");
+	switch (Game::theGame->Turn())
+	{
+		case Piece::RED:
+			s << Game::theGame->red->name << " RED ";
+			break;
+		case Piece::BLUE:
+			s << Game::theGame->blue->name << " BLUE ";
+			break;
+		case Piece::BOTH:
+			s << "neither BOTH ";
+			break;
+		case Piece::NONE:
+			s << "neither NONE ";
+			break;
+	}
+
+	if (!Board::LegalResult(result) && result != MovementResult::BAD_SETUP)
+		s << "ILLEGAL ";
+	else if (!Board::HaltResult(result))
+		s << "INTERNAL_ERROR ";
+	else
+	{
+		switch (result.type)
+		{
+			case MovementResult::VICTORY_FLAG:
+			case MovementResult::VICTORY_ATTRITION: //It does not matter how you win, it just matters that you won!
+				s <<  "VICTORY ";
+				break;
+			case MovementResult::SURRENDER:
+				s << "SURRENDER ";
+				break;
+			case MovementResult::DRAW:
+				s << "DRAW ";
+				break;
+			case MovementResult::DRAW_DEFAULT:
+				s << "DRAW_DEFAULT ";
+				break;
+			case MovementResult::BAD_SETUP:
+				s << "BAD_SETUP ";
+				break;	
+			default:
+				s << "INTERNAL_ERROR ";
+				break;	
+		}
+	}
+	
+	s << Game::theGame->TurnCount() << " " << Game::theGame->theBoard.TotalPieceValue(Piece::RED) << " " << Game::theGame->theBoard.TotalPieceValue(Piece::BLUE);
+
+	buffer = s.str();
+	
+
+}
+
diff --git a/judge/manager/game.h b/judge/manager/game.h
index 5a5f8a6..ce83545 100644
--- a/judge/manager/game.h
+++ b/judge/manager/game.h
@@ -43,6 +43,7 @@ class Game
 		void MakeControllers(const char * redPath, const char * bluePath); //Create a controller based off a path
 	public:
 		int logMessage(const char * format, ...);
+		static void PrintResults(const MovementResult & result, std::string & buffer);
 		FILE * GetLogFile() const {return log;}
 		Controller * red;
 		Controller * blue;
@@ -83,7 +84,7 @@ class FileController : public Controller
 		FileController(const Piece::Colour & newColour, FILE * newFile) : Controller(newColour, "file"), file(newFile) {}
 		virtual ~FileController() {}
 
-		virtual void Message(const char * string) {} //Don't send messages
+		virtual bool Message(const char * string) {return true;} //Don't send messages
 		virtual MovementResult QuerySetup(const char * opponentName, std::string setup[]);
 		virtual MovementResult QueryMove(std::string & buffer);
 		virtual bool Valid() const {return file != NULL;}
diff --git a/judge/manager/human_controller.h b/judge/manager/human_controller.h
index b2069fc..9a53ac4 100644
--- a/judge/manager/human_controller.h
+++ b/judge/manager/human_controller.h
@@ -15,7 +15,7 @@ class Human_Controller : public Controller
 		virtual bool HumanController() const {return true;}
 		virtual MovementResult QuerySetup(const char * opponentName, std::string setup[]);
 		virtual MovementResult QueryMove(std::string & buffer); 
-		virtual void Message(const char * message) {fprintf(stderr, "%s\n", message);}
+		virtual bool Message(const char * message) {return (strlen(message) <= 0 || fprintf(stderr, "%s\n", message) > 0);}
 	
 	private:
 		const bool graphicsEnabled;
diff --git a/judge/manager/main.cpp b/judge/manager/main.cpp
index b0e2b4c..ee35837 100644
--- a/judge/manager/main.cpp
+++ b/judge/manager/main.cpp
@@ -13,7 +13,7 @@ using namespace std;
 
 Piece::Colour SetupGame(int argc, char ** argv);
 void DestroyGame();
-void PrintResults(const MovementResult & result, string & buffer);
+
 
 char * video = NULL;
 
@@ -46,7 +46,7 @@ int main(int argc, char ** argv)
 	Game::theGame->PrintEndMessage(result);
 
 	string buffer = "";
-	PrintResults(result, buffer);
+	Game::PrintResults(result, buffer);
 
 	//Message the AI's the quit message
 	Game::theGame->red->Message("QUIT " + buffer);
@@ -278,61 +278,6 @@ Piece::Colour SetupGame(int argc, char ** argv)
 
 }
 
-void PrintResults(const MovementResult & result, string & buffer)
-{
-	stringstream s("");
-	switch (Game::theGame->Turn())
-	{
-		case Piece::RED:
-			s << Game::theGame->red->name << " RED ";
-			break;
-		case Piece::BLUE:
-			s << Game::theGame->blue->name << " BLUE ";
-			break;
-		case Piece::BOTH:
-			s << "neither BOTH ";
-			break;
-		case Piece::NONE:
-			s << "neither NONE ";
-			break;
-	}
-
-	if (!Board::LegalResult(result) && result != MovementResult::BAD_SETUP)
-		s << "ILLEGAL ";
-	else if (!Board::HaltResult(result))
-		s << "INTERNAL_ERROR ";
-	else
-	{
-		switch (result.type)
-		{
-			case MovementResult::VICTORY_FLAG:
-			case MovementResult::VICTORY_ATTRITION: //It does not matter how you win, it just matters that you won!
-				s <<  "VICTORY ";
-				break;
-			case MovementResult::SURRENDER:
-				s << "SURRENDER ";
-				break;
-			case MovementResult::DRAW:
-				s << "DRAW ";
-				break;
-			case MovementResult::DRAW_DEFAULT:
-				s << "DRAW_DEFAULT ";
-				break;
-			case MovementResult::BAD_SETUP:
-				s << "BAD_SETUP ";
-				break;	
-			default:
-				s << "INTERNAL_ERROR ";
-				break;	
-		}
-	}
-	
-	s << Game::theGame->TurnCount() << " " << Game::theGame->theBoard.TotalPieceValue(Piece::RED) << " " << Game::theGame->theBoard.TotalPieceValue(Piece::BLUE);
-
-	buffer = s.str();
-	
-
-}
 
 void DestroyGame()
 {
diff --git a/judge/manager/network_controller.h b/judge/manager/network_controller.h
index 31b0c92..130bede 100644
--- a/judge/manager/network_controller.h
+++ b/judge/manager/network_controller.h
@@ -27,10 +27,10 @@ class NetworkSender : public NetworkController
 
 		virtual bool Valid() const {return NetworkController::Valid() && controller->Valid();}
 
-		virtual void Message(const char * message) 
+		virtual bool Message(const char * message) 
 		{
 			//fprintf(stderr,"NetworkSender sending message %s to underlying controller\n", message);
-			controller->Message(message);
+			return (controller->Message(message));
 		}
 
 		virtual MovementResult QuerySetup(const char * opponentName, std::string setup[]);
@@ -47,7 +47,7 @@ class NetworkReceiver : public NetworkController
 		NetworkReceiver(const Piece::Colour & colour, Network * newNetwork) : NetworkController(colour, newNetwork) {}
 		virtual ~NetworkReceiver() {}
 
-		virtual void Message(const char * message) {} //Do nothing (Counter intuitive much)
+		virtual bool Message(const char * message) {return true;} //Do nothing (Counter intuitive much)
 		virtual MovementResult QuerySetup(const char * opponentName, std::string setup[]);
 		virtual MovementResult QueryMove(std::string & buffer);
 		
diff --git a/judge/manager/program.cpp b/judge/manager/program.cpp
index c936de3..00a28e9 100644
--- a/judge/manager/program.cpp
+++ b/judge/manager/program.cpp
@@ -251,6 +251,7 @@ bool Program::GetMessage(string & buffer, double timeout)
 		buffer += c;
 	}
 	//fprintf(stderr, "%s\n", buffer.c_str());
+	//fprintf(stderr,"DONE\n");
 	return true;
 
 	/* Old way, using threads, which apparently is terrible
-- 
GitLab