Merge pull request #8 from tgstation/ConnectionTimeouts

Specify the timeout

src/BSQL/API.cpp

@@ -90,12 +90,18 @@ extern "C" {
 	}
 
 	BYOND_FUNC CreateConnection(const int argumentCount, const char* const* const args) noexcept {
-		if (argumentCount != 1)
+		if (argumentCount != 3)
 			return "Invalid arguments!";
 		if (!library)
 			return "Library not initialized!";
 		const auto& connectionType(args[0]);
+		const auto& asyncTimeoutStr(args[1]);
+		const auto& blockingTimeoutStr(args[2]);
 		Connection::Type type;
+
+		const auto asyncTimeout(std::atoi(asyncTimeoutStr));
+		const auto blockingTimeout(std::atoi(blockingTimeoutStr));
+
 		try {
 			std::string conType(connectionType);
 			if (conType == "MySql")
@@ -110,11 +116,19 @@ extern "C" {
 			return "Out of memory!";
 		}
 
+		if (asyncTimeout < 0)
+			return "asyncTimeout must be an unsigned integer!";
+		if (blockingTimeout < 0)
+			return "blockingTimeout must be an unsigned integer!";
+
+		if (asyncTimeout != 0 && blockingTimeout > asyncTimeout)
+			return "asyncTimeout must be greater than or equal to blockingTimeout";
+
 		if (!lastCreatedConnection.empty())
 			//guess they didn't want it
 			library->ReleaseConnection(lastCreatedConnection);
 
-		auto result(library->CreateConnection(type));
+		auto result(library->CreateConnection(type, static_cast<unsigned int>(asyncTimeout), static_cast<unsigned int>(blockingTimeout)));
 		if (result.empty())
 			return "Out of memory";
 
@@ -339,8 +353,11 @@ extern "C" {
 			auto op(connection->GetOperation(operationIdentifier));
 			if (!op)
 				return "Operation identifier does not exist!";
-			while (!op->IsComplete(false))
+			auto I(0U);
+			for (; !op->IsComplete(false) && I < connection->blockingTimeout * 1000; ++I)
 				std::this_thread::sleep_for(std::chrono::milliseconds(1));
+			if (I >= connection->blockingTimeout * 1000)
+				return "Operation timed out!";	//match this with the api, too lazy to do it any other way
 			return nullptr;
 		}
 		catch (std::bad_alloc&) {

src/BSQL/Connection.cpp

@@ -1,6 +1,7 @@
 #include "BSQL.h"
 
-Connection::Connection(Type type, Library& library) :
+Connection::Connection(Type type, Library& library, const unsigned int blockingTimeout) :
+	blockingTimeout(blockingTimeout),
 	library(library),
 	type(type),
 	identifierCounter(0)

src/BSQL/Connection.h

@@ -7,14 +7,15 @@ class Connection {
 		SqlServer
 	};
 public:
+	const unsigned int blockingTimeout;
 	const Type type;
 protected:
 	Library & library;
 	std::map<std::string, std::unique_ptr<Operation>> operations;
 private:
 	unsigned long long identifierCounter;
 protected:
-	Connection(Type type, Library& library);
+	Connection(Type type, Library& library, const unsigned int blockingTimeout);
 
 	std::string AddOp(std::unique_ptr<Operation>&& operation);
 public:

src/BSQL/Library.cpp

@@ -24,15 +24,15 @@ bool Library::ReleaseConnection(const std::string& identifier) noexcept {
 	return connections.erase(identifier) > 0;
 }
 
-std::string Library::CreateConnection(Connection::Type type) noexcept {
+std::string Library::CreateConnection(Connection::Type type, const unsigned int asyncTimeout, const unsigned int blockingTimeout) noexcept {
 	if (identifierCounter < std::numeric_limits<unsigned long long>().max()) {
 		try {
 			auto identifier(std::to_string(++identifierCounter));
 
 			switch (type)
 			{
 			case Connection::Type::MySql:
-				connections.emplace(identifier, std::make_unique<MySqlConnection>(*this));
+				connections.emplace(identifier, std::make_unique<MySqlConnection>(*this, asyncTimeout, blockingTimeout));
 				break;
 			case Connection::Type::SqlServer:
 				--identifierCounter;

src/BSQL/Library.h

@@ -10,7 +10,7 @@ class Library {
 	Library() noexcept;
 	~Library() noexcept;
 
-	std::string CreateConnection(Connection::Type connectionType) noexcept;
+	std::string CreateConnection(Connection::Type connectionType, const unsigned int asyncTimeout, const unsigned int blockingTimeout) noexcept;
 	Connection* GetConnection(const std::string& identifier) noexcept;
 	bool ReleaseConnection(const std::string& identifier) noexcept;
 	void RegisterZombieThread(std::thread&& thread) noexcept;

src/BSQL/MySqlConnectOperation.cpp

@@ -1,18 +1,21 @@
 #include "BSQL.h"
 
-MySqlConnectOperation::MySqlConnectOperation(MySqlConnection& connPool, const std::string& address, const unsigned short port, const std::string& username, const std::string& password, const std::string& database) :
+MySqlConnectOperation::MySqlConnectOperation(MySqlConnection& connPool, const std::string& address, const unsigned short port, const std::string& username, const std::string& password, const std::string& database, const unsigned int timeout) :
 	connPool(connPool),
 	mysql(nullptr),
 	complete(false),
 	state(std::make_shared<ClassState>()),
-	connectThread(&MySqlConnectOperation::DoConnect, this, address, port, username, password, database, InitMySql(), state)
+	connectThread(&MySqlConnectOperation::DoConnect, this, address, port, username, password, database, InitMySql(timeout), state)
 {
 }
 
-MYSQL* MySqlConnectOperation::InitMySql() {
+MYSQL* MySqlConnectOperation::InitMySql(const unsigned int timeout) {
 	const auto res(mysql_init(nullptr));
 	if (!res)
 		throw std::bad_alloc();
+	mysql_options(res, MYSQL_OPT_CONNECT_TIMEOUT, static_cast<const void*>(&timeout));
+	mysql_options(res, MYSQL_OPT_READ_TIMEOUT, static_cast<const void*>(&timeout));
+	mysql_options(res, MYSQL_OPT_WRITE_TIMEOUT, static_cast<const void*>(&timeout));
 	return res;
 }
 

src/BSQL/MySqlConnectOperation.h

@@ -8,11 +8,12 @@ class MySqlConnectOperation : public Operation {
 	bool complete;
 	std::shared_ptr<ClassState> state;
 	std::thread connectThread;
+
 private:
-	static MYSQL* InitMySql();
+	static MYSQL* InitMySql(const unsigned int timeout);
 	void DoConnect(const std::string address, const unsigned short port, const std::string username, const std::string password, const std::string database, MYSQL* localMySql, std::shared_ptr<ClassState> localState);
 public:
-	MySqlConnectOperation(MySqlConnection& connPool, const std::string& address, const unsigned short port, const std::string& username, const std::string& password, const std::string& database);
+	MySqlConnectOperation(MySqlConnection& connPool, const std::string& address, const unsigned short port, const std::string& username, const std::string& password, const std::string& database, const unsigned int timeout);
 	MySqlConnectOperation(const MySqlConnectOperation&) = delete;
 	MySqlConnectOperation(MySqlConnectOperation&&) = delete;
 	~MySqlConnectOperation() override = default;

src/BSQL/MySqlConnection.cpp

@@ -1,8 +1,9 @@
 #include "BSQL.h"
 
-MySqlConnection::MySqlConnection(Library& library) :
-	Connection(Type::MySql, library),
-	firstSuccessfulConnection(nullptr)
+MySqlConnection::MySqlConnection(Library& library, const unsigned int asyncTimeout, const unsigned int blockingTimeout) :
+	Connection(Type::MySql, library, blockingTimeout),
+	firstSuccessfulConnection(nullptr),
+	asyncTimeout(asyncTimeout)
 {}
 
 MySqlConnection::~MySqlConnection() {
@@ -62,7 +63,7 @@ bool MySqlConnection::LoadNewConnection(std::string& fail, int& failno) {
 			return false;
 	}
 
-	newestConnectionAttemptKey = AddOp(std::make_unique<MySqlConnectOperation>(*this, address, port, username, password, database));
+	newestConnectionAttemptKey = AddOp(std::make_unique<MySqlConnectOperation>(*this, address, port, username, password, database, asyncTimeout));
 
 	return false;
 }

src/BSQL/MySqlConnection.h

@@ -8,15 +8,17 @@ class MySqlConnection : public Connection {
 	std::string username;
 	std::string password;
 	std::string database;
-	unsigned short port;
 
 	std::stack<MYSQL*> availableConnections;
 	MYSQL* firstSuccessfulConnection;
 	std::string newestConnectionAttemptKey;
+
+	const unsigned int asyncTimeout;
+	unsigned short port;
 private:
 	bool LoadNewConnection(std::string& fail, int& failno);
 public:
-	MySqlConnection(Library& library);
+	MySqlConnection(Library& library, const unsigned int asyncTimeout, const unsigned int blockingTimeout);
 	~MySqlConnection() override;
 
 	std::string Connect(const std::string& address, const unsigned short port, const std::string& username, const std::string& password, const std::string& database) override;

src/DMAPI/BSQL.dm

@@ -1,9 +1,11 @@
-//BSQL - DMAPI v1.1.1.0
+//BSQL - DMAPI v1.2.0.0
 
 //types of connections
 #define BSQL_CONNECTION_TYPE_MARIADB "MySql"
 #define BSQL_CONNECTION_TYPE_SQLSERVER "SqlServer"
 
+#define BSQL_DEFAULT_TIMEOUT 5
+
 //Call this before rebooting or shutting down your world to clean up gracefully. This invalidates all active connection and operation datums
 /world/proc/BSQL_Shutdown()
 	return
@@ -18,8 +20,10 @@ Called whenever a library call is made with verbose information, override and do
 /*
 Create a new database connection, does not perform the actual connect
   connection_type: The BSQL connection_type to use
+  asyncTimeout: The timeout to use for normal operations, 0 for infinite, defaults to BSQL_DEFAULT_TIMEOUT
+  blockingTimeout: The timeout to use for blocking operations, must be less than or equal to asyncTimeout, 0 for infinite, defaults to asyncTimeout
 */
-/datum/BSQL_Connection/New(connection_type)
+/datum/BSQL_Connection/New(connection_type, asyncTimeout, blockingTimeout)
 	return ..()
 
 /*
@@ -61,7 +65,9 @@ Checks if the operation is complete. This, in some cases must be called multiple
 	return
 
 /*
-Blocks the entire game until the given operation completes. IsComplete should not be checked after calling this to avoid potential side effects
+Blocks the entire game until the given operation completes. IsComplete should not be checked after calling this to avoid potential side effects.
+
+Returns: TRUE on success, FALSE if the operation wait time exceeded the connection's blockingTimeout setting
 */
 /datum/BSQL_Operation/proc/WaitForCompletion()
 	return

src/DMAPI/BSQL/core/connection.dm

@@ -4,12 +4,17 @@
 
 BSQL_PROTECT_DATUM(/datum/BSQL_Connection)
 
-/datum/BSQL_Connection/New(connection_type)
+/datum/BSQL_Connection/New(connection_type, asyncTimeout, blockingTimeout)
+	if(asyncTimeout == null)
+		asyncTimeout = BSQL_DEFAULT_TIMEOUT
+	if(blockingTimeout == null)
+		blockingTimeout = asyncTimeout
+
 	src.connection_type = connection_type
 
 	world._BSQL_InitCheck(src)
 
-	var/error = world._BSQL_Internal_Call("CreateConnection", connection_type)
+	var/error = world._BSQL_Internal_Call("CreateConnection", connection_type, "[asyncTimeout]", "[blockingTimeout]")
 	if(error)
 		BSQL_ERROR(error)
 		return

src/DMAPI/BSQL/core/operation.dm

@@ -40,4 +40,8 @@ BSQL_DEL_PROC(/datum/BSQL_Operation)
 		return
 	var/error = world._BSQL_Internal_Call("BlockOnOperation", connection.id, id)
 	if(error)
+		if(error == "Operation timed out!")	//match this with the implementation
+			return FALSE
 		BSQL_ERROR("Error waiting for operation [id] for connection [connection.id]! [error]")
+		return
+	return TRUE

tests/Test.dm

@@ -37,7 +37,7 @@
 
 /proc/Test()
 	world.log << "Beginning test"
-
+	
 	var/host = world.params["dbhost"]
 	var/user = world.params["dbuser"]
 	var/port = text2num(world.params["dbport"])
@@ -84,7 +84,8 @@
 
 	q = conn.BeginQuery("CREATE DATABASE [quoted_db]");
 	world.log << "Create db op id: [q.id]"
-	q.WaitForCompletion()
+	if(!q.WaitForCompletion())
+		CRASH("WaitForCompletion timed out")
 	if(!q.IsComplete())
 		CRASH("Wait for completion didn't work!")
 	error = q.GetError()
@@ -161,6 +162,13 @@
 	results = q.CurrentRow()
 	if(results)
 		CRASH("Expected no third row! Got: [json_encode(results)] !")
+
+	q = conn.BeginQuery("LOCK TABLES asdf WRITE")
+	world.log << "Lock query id: [q.id]"
+	WaitOp(q)
+	error = q.GetError()
+	if(error)
+		CRASH(error)
 
 	del(q)
 	del(conn)