Refactor RDNS resolution; update Jupiter; bug fixes

baseline/Configs/RenX.Commands.ini

@@ -2,7 +2,7 @@
 ;
 ; DefaultTBanTime=Integer (Default: 1d; 1 day)
 ; MaxTBanTime=Integer (Default: 1w; 1 week)
-; PlayerInfoFormat=String (Default: 03[Player Info]{TCOLOR} Name: {RNAME} - ID: {ID} - Team: {TEAML} - Vehicle Kills: {VEHICLEKILLS} - Building Kills {BUILDINGKILLS} - Kills {KILLS} - Deaths: {DEATHS} - KDR: {KDR} - Access: {ACCESS}")
+; PlayerInfoFormat=String (Default: 03[Player Info]{TCOLOR} {RNAME} - ID: {ID} - Team: {TEAML} - Vehicle Kills: {VEHICLEKILLS} - Building Kills {BUILDINGKILLS} - Kills {KILLS} - Deaths: {DEATHS} - KDR: {KDR} - Access: {ACCESS}")
 ; AdminPlayerInfoFormat=String (Default: PlayerInfoFormat - IP: {IP} - Steam ID: {STEAM})
 ; BuildingInfoFormat=String (Default: {BCOLOR} {BNAME} - 07{BHP}%)
 ; StaffTitle=String (Default: Moderator)

src/Bot/src/Main.cpp

@@ -24,6 +24,7 @@
 #include <thread>
 #include <mutex>
 #include "jessilib/unicode.hpp"
+#include "jessilib/app_parameters.hpp"
 #include "Jupiter/Functions.h"
 #include "Jupiter/INIConfig.h"
 #include "Jupiter/Socket.h"
@@ -39,6 +40,7 @@
 #endif // _WIN32
 
 using namespace Jupiter::literals;
+using namespace std::literals;
 
 Jupiter::INIConfig o_config;
 Jupiter::Config *Jupiter::g_config = &o_config;
@@ -163,11 +165,10 @@ void reinitialize_plugins() {
 	}
 }
 
-int main(int argc, const char **args) {
+int main(int argc, char* argv[]) {
 	atexit(onExit);
 	std::set_terminate(onTerminate);
 	std::thread inputThread(inputLoop);
-	Jupiter::ReferenceString plugins_directory, configs_directory;
 
 #if defined SIGPIPE
 	std::signal(SIGPIPE, SIG_IGN);
@@ -180,33 +181,24 @@ int main(int argc, const char **args) {
 #endif // _WIN32
 
 	srand(static_cast<unsigned int>(std::chrono::system_clock::now().time_since_epoch().count()));
-	puts(Jupiter::copyright);
-	const char *configFileName = "Config.ini";
-
-	for (int i = 1; i < argc; i++) {
-		std::string_view arg_view = args[i];
-		if (jessilib::equalsi("-help"_jrs, arg_view)) {
-			puts("Help coming soon, to a theatre near you!");
-			return 0;
-		}
-		else if (jessilib::equalsi("-config"_jrs, arg_view) && ++i < argc)
-			configFileName = args[i];
-		else if (jessilib::equalsi("-pluginsdir"_jrs, arg_view) && ++i < argc)
-			plugins_directory = arg_view;
-		else if (jessilib::equalsi("-configsdir"_jrs, arg_view) && ++i < argc)
-			configs_directory = arg_view;
-		else if (jessilib::equalsi("-configFormat"_jrs, arg_view) && ++i < argc)
-			puts("Feature not yet supported!");
-		else
-			printf("Warning: Unknown command line argument \"%s\" specified. Ignoring...", args[i]);
+	std::cout << Jupiter::copyright << std::endl;
+
+	jessilib::app_parameters parameters{ argc, argv };
+	if (parameters.has_switch("help"sv)) {
+		std::cout << "Help coming soon, to a theatre near you!" << std::endl;
+		return 0;
 	}
 
+	std::string_view configFileName = parameters.get_value("config", "Config.ini"sv);
+	std::string_view plugins_directory = parameters.get_value("pluginsdir"sv);
+	std::string_view configs_directory = parameters.get_value("configsdir"sv);
+
 	std::chrono::steady_clock::time_point load_start = std::chrono::steady_clock::now();
 
-	puts("Loading config file...");
+	std::cout << "Loading config file..." << std::endl;
 	if (!o_config.read(configFileName)) {
-		puts("Unable to read config file. Closing...");
-		exit(0);
+		std::cout << "Unable to read config file. Closing..." << std::endl;
+		return 1;
 	}
 
 	double time_taken = static_cast<double>(std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::steady_clock::now() - load_start).count()) / 1000.0;

src/Jupiter

@@ -1 +1 @@
-Subproject commit 6b693b96a9f77d1ec49e11d991d9c5a75965f4c4
+Subproject commit e4dc793c42ab7bbd1ca896d1c10d9f63244011b6

src/Plugins/RenX/RenX.Commands/RenX_Commands.cpp

@@ -79,9 +79,8 @@ bool RenX_CommandsPlugin::initialize() {
 	auto max_tban_time = this->config.get("MaxTBanTime"_jrs, "1w"_jrs);
 	m_defaultTempBanTime = jessilib::duration_from_string(default_tban_time.data(), default_tban_time.data() + default_tban_time.size()).duration;
 	m_maxTempBanTime = std::max(jessilib::duration_from_string(max_tban_time.data(), max_tban_time.data() + max_tban_time.size()).duration, m_defaultTempBanTime);
-	m_playerInfoFormat = this->config.get("PlayerInfoFormat"_jrs, IRCCOLOR "03[Player Info]" IRCCOLOR "{TCOLOR} Name: " IRCBOLD "{RNAME}" IRCBOLD " - ID: {ID} - Team: " IRCBOLD "{TEAML}" IRCBOLD " - Vehicle Kills: {VEHICLEKILLS} - Building Kills {BUILDINGKILLS} - Kills {KILLS} - Deaths: {DEATHS} - KDR: {KDR} - Access: {ACCESS}"_jrs);
-	m_adminPlayerInfoFormat = this->config.get("AdminPlayerInfoFormat"_jrs, Jupiter::StringS::Format("%.*s - IP: " IRCBOLD "{IP}" IRCBOLD " - HWID: " IRCBOLD "{HWID}" IRCBOLD " - RDNS: " IRCBOLD "{RDNS}" IRCBOLD " - Steam ID: " IRCBOLD "{STEAM}", m_playerInfoFormat.size(),
-		m_playerInfoFormat.data()));
+	m_playerInfoFormat = this->config.get("PlayerInfoFormat"_jrs, IRCCOLOR "03[Player Info]" IRCCOLOR "{TCOLOR} " IRCBOLD "{RNAME}" IRCBOLD " - ID: {ID} - Team: " IRCBOLD "{TEAML}" IRCBOLD " - Vehicle Kills: {VEHICLEKILLS} - Building Kills {BUILDINGKILLS} - Kills {KILLS} - Deaths: {DEATHS} - KDR: {KDR} - Access: {ACCESS}"_jrs);
+	m_adminPlayerInfoFormat = this->config.get("AdminPlayerInfoFormat"_jrs, m_playerInfoFormat + " - IP: " IRCBOLD "{IP}" IRCBOLD " - HWID: " IRCBOLD "{HWID}" IRCBOLD " - RDNS: " IRCBOLD "{RDNS}" IRCBOLD " - Steam ID: " IRCBOLD "{STEAM}");
 	m_buildingInfoFormat = this->config.get("BuildingInfoFormat"_jrs, ""s IRCCOLOR + RenX::tags->buildingTeamColorTag + RenX::tags->buildingNameTag + IRCCOLOR " - " IRCCOLOR "07"_jrs + RenX::tags->buildingHealthPercentageTag + "%"_jrs);
 	m_staffTitle = this->config.get("StaffTitle"_jrs, "Moderator"_jrs);
 

src/Plugins/RenX/RenX.Core/RenX_BanDatabase.cpp

@@ -171,9 +171,7 @@ void RenX::BanDatabase::add(RenX::Server *server, const RenX::PlayerInfo &player
 	if (player.hwid.find_first_not_of('0') != std::string::npos) {
 		entry->hwid = player.hwid;
 	}
-	if (player.rdns_thread.joinable())
-		player.rdns_thread.join();
-	entry->rdns = player.rdns;
+	entry->rdns = player.get_rdns();
 	entry->name = player.name;
 	entry->banner = banner;
 	entry->reason = reason;

src/Plugins/RenX/RenX.Core/RenX_PlayerInfo.h

@@ -55,9 +55,7 @@ namespace RenX
 		std::string uuid;
 		Jupiter::StringS character;
 		Jupiter::StringS vehicle;
-		std::string rdns;
 		std::string hwid;
-		std::mutex rdns_mutex;
 		uint64_t steamid = 0;
 		uint32_t ip32 = 0;
 		uint16_t ban_flags = 0;
@@ -66,7 +64,6 @@ namespace RenX
 		int id = 0;
 		bool isBot = false;
 		bool is_dev = false;
-		bool rdns_resolved = false;
 		unsigned short ping = 0;
 		double score = 0.0f;
 		double credits = 0.0f;
@@ -89,11 +86,30 @@ namespace RenX
 		unsigned int steals = 0;
 		unsigned int stolen = 0;
 
+		// Lock-free getter -- never access m_rdns until it's been set by RDNS thread
+		std::string_view get_rdns() const {
+			if (m_rdns_ptr.use_count() != 1
+				|| m_rdns_ptr == nullptr) {
+				return {};
+			}
+
+			// In theory if get_rdns() were ever called at the same time as start_resolve_rdns(), on separate threads,
+			// then there would be a race condition here causing undefined behavior. However, we only interact with
+			// players on a single thread, and even beyond that, we only call start_resolve_rdns() immediately after
+			// constructing PlayerInfo. Therefore, this is safe
+			return *m_rdns_ptr;
+		}
+		static void resolve_rdns(std::string in_ip, std::shared_ptr<std::string> out_rdns);
+		void start_resolve_rdns();
+		bool rdns_pending = false;
+		
 		mutable Jupiter::StringS gamePrefix;
 		mutable Jupiter::StringS formatNamePrefix;
-		mutable std::thread rdns_thread;
 		mutable int access = 0;
-		mutable Jupiter::Config varData;
+		mutable Jupiter::Config varData; // TODO: use jessilib::object instead
+
+	private:
+		std::shared_ptr<std::string> m_rdns_ptr; // Needs synchronization across threads
 	};
 
 	static std::string_view rdns_pending{ "RDNS_PENDING" };

src/Plugins/RenX/RenX.Core/RenX_Server.cpp

@@ -65,24 +65,23 @@ int RenX::Server::think() {
 	else {
 		auto cycle_player_rdns = [this]() { // Cycles through any pending RDNS resolutions, and fires events as necessary.
 			if (m_player_rdns_resolutions_pending != 0) {
-				for (auto node = this->players.begin(); node != this->players.end(); ++node) {
-					if (node->rdns_thread.joinable() && node->rdns_mutex.try_lock()) { // RDNS event hasn't fired AND RDNS value has been resolved
-						node->rdns_mutex.unlock();
-						node->rdns_thread.join();
+				for (auto& player : players) {
+					if (player.rdns_pending && !player.get_rdns().empty()) { // RDNS event hasn't fired AND RDNS value has been resolved
+						player.rdns_pending = false;
 						--m_player_rdns_resolutions_pending;
 
 						// Check for bans
-						banCheck(*node);
+						banCheck(player);
 
 						// Fire RDNS resolved event
 						for (const auto& plugin : RenX::getCore()->getPlugins()) {
-							plugin->RenX_OnPlayerRDNS(*this, *node);
+							plugin->RenX_OnPlayerRDNS(*this, player);
 						}
 
 						// Fire player indentified event if ready
-						if (!node->hwid.empty()) {
+						if (!player.hwid.empty()) {
 							for (auto plugin : RenX::getCore()->getPlugins()) {
-								plugin->RenX_OnPlayerIdentify(*this, *node);
+								plugin->RenX_OnPlayerIdentify(*this, player);
 							}
 						}
 
@@ -555,7 +554,7 @@ void RenX::Server::banCheck(RenX::PlayerInfo &player) {
 				if ((m_localSteamBan && entry->steamid != 0 && entry->steamid == player.steamid)
 					|| (m_localIPBan && entry->ip != 0 && (entry->ip & netmask) == (player.ip32 & netmask))
 					|| (m_localHWIDBan && !entry->hwid.empty() && entry->hwid == player.hwid)
-					|| (m_localRDNSBan && !entry->rdns.empty() && entry->is_rdns_ban() && !player.rdns_thread.joinable() && player.rdns.find(entry->rdns) != std::string::npos)
+					|| (m_localRDNSBan && !entry->rdns.empty() && entry->is_rdns_ban() && player.get_rdns().find(entry->rdns) != std::string::npos)
 					|| (m_localNameBan && !entry->name.empty() && jessilib::equalsi(entry->name, player.name))) {
 					player.ban_flags |= entry->flags;
 					if (entry->is_type_game())
@@ -735,9 +734,8 @@ bool RenX::Server::removePlayer(int id) {
 				--m_bot_count;
 			}
 
-			if (node->rdns_thread.joinable()) { // Close the RDNS thread, if one exists
+			if (node->rdns_pending) {
 				--m_player_rdns_resolutions_pending;
-				node->rdns_thread.join();
 			}
 
 			this->players.erase(node);
@@ -1302,14 +1300,17 @@ void RenX::Server::sendLogChan(std::string_view msg) const {
 	}
 }
 
-void resolve_rdns(RenX::PlayerInfo *player) {
-	player->rdns_mutex.lock();
+void RenX::PlayerInfo::resolve_rdns(std::string in_ip, std::shared_ptr<std::string> out_rdns) {
+	*out_rdns = Jupiter::Socket::resolveHostname(in_ip.c_str(), 0);
+}
+
+void RenX::PlayerInfo::start_resolve_rdns() {
+	rdns_pending = true;
 
-	// TODO: don't do this
-	char *resolved = Jupiter::Socket::resolveHostname_alloc(static_cast<std::string>(player->ip).c_str(), 0);
-	player->rdns = resolved;
-	delete[] resolved;
-	player->rdns_mutex.unlock();
+	std::string player_ip = ip;
+	auto rdns_string = std::make_shared<std::string>();
+	m_rdns_ptr = rdns_string;
+	std::thread(resolve_rdns, ip, std::move(rdns_string)).detach();
 }
 
 struct parsed_player_token {
@@ -1519,7 +1520,7 @@ void RenX::Server::processLine(std::string_view line) {
 			// RDNS
 			if (resolvesRDNS() && player->ip32 != 0)
 			{
-				player->rdns_thread = std::thread(resolve_rdns, player);
+				player->start_resolve_rdns();
 				++m_player_rdns_resolutions_pending;
 			}
 
@@ -1555,7 +1556,7 @@ void RenX::Server::processLine(std::string_view line) {
 				player->ip32 = Jupiter::Socket::pton4(static_cast<std::string>(player->ip).c_str());
 				if (resolvesRDNS())
 				{
-					player->rdns_thread = std::thread(resolve_rdns, player);
+					player->start_resolve_rdns();
 					++m_player_rdns_resolutions_pending;
 				}
 				recalcUUID = true;
@@ -2847,8 +2848,7 @@ void RenX::Server::processLine(std::string_view line) {
 								}
 						}
 					}
-					else if (subHeader == "HWID;"sv)
-					{
+					else if (subHeader == "HWID;"sv) {
 						// ["player" |] Player | "hwid" | HWID
 						size_t offset = 0;
 						if (getToken(2) == "player"sv)
@@ -2865,7 +2865,7 @@ void RenX::Server::processLine(std::string_view line) {
 							plugin->RenX_OnHWID(*this, *player);
 						}
 
-						if (!player->rdns.empty()) {
+						if (!player->rdns_pending) {
 							for (const auto& plugin : xPlugins) {
 								plugin->RenX_OnPlayerIdentify(*this, *player);
 							}
@@ -3654,14 +3654,10 @@ void RenX::Server::wipePlayers() {
 		for (const auto& plugin : RenX::getCore()->getPlugins()) {
 			plugin->RenX_OnPlayerDelete(*this, this->players.front());
 		}
-
-		if (this->players.front().rdns_thread.joinable()) { // Close the RDNS thread, if one exists
-			--m_player_rdns_resolutions_pending;
-			this->players.front().rdns_thread.join();
-		}
-
 		this->players.pop_front();
 	}
+
+	m_player_rdns_resolutions_pending = 0;
 }
 
 void RenX::Server::startPing() {

src/Plugins/RenX/RenX.Core/RenX_Tags.cpp

@@ -477,13 +477,11 @@ void TagsImp::processTags(std::string& msg, const RenX::Server *server, const Re
 		PROCESS_TAG(this->INTERNAL_RAW_NAME_TAG, player->name);
 		PROCESS_TAG(this->INTERNAL_IP_TAG, player->ip);
 		PROCESS_TAG(this->INTERNAL_HWID_TAG, player->hwid);
-		if (player->rdns_thread.joinable())
-		{
+		if (player->rdns_pending) {
 			PROCESS_TAG(this->INTERNAL_RDNS_TAG, RenX::rdns_pending);
 		}
-		else
-		{
-			PROCESS_TAG(this->INTERNAL_RDNS_TAG, player->rdns);
+		else {
+			PROCESS_TAG(this->INTERNAL_RDNS_TAG, player->get_rdns());
 		}
 		PROCESS_TAG(this->INTERNAL_UUID_TAG, player->uuid);
 		PROCESS_TAG(this->INTERNAL_ID_TAG, Jupiter::StringS::Format("%d", player->id));
@@ -523,13 +521,11 @@ void TagsImp::processTags(std::string& msg, const RenX::Server *server, const Re
 		PROCESS_TAG(this->INTERNAL_VICTIM_RAW_NAME_TAG, victim->name);
 		PROCESS_TAG(this->INTERNAL_VICTIM_IP_TAG, victim->ip);
 		PROCESS_TAG(this->INTERNAL_VICTIM_HWID_TAG, victim->hwid);
-		if (victim->rdns_thread.joinable())
-		{
+		if (victim->rdns_pending) {
 			PROCESS_TAG(this->INTERNAL_VICTIM_RDNS_TAG, RenX::rdns_pending);
 		}
-		else
-		{
-			PROCESS_TAG(this->INTERNAL_VICTIM_RDNS_TAG, victim->rdns);
+		else {
+			PROCESS_TAG(this->INTERNAL_VICTIM_RDNS_TAG, victim->get_rdns());
 		}
 		PROCESS_TAG(this->INTERNAL_VICTIM_UUID_TAG, victim->uuid);
 		PROCESS_TAG(this->INTERNAL_VICTIM_ID_TAG, Jupiter::StringS::Format("%d", victim->id));