Qdels queries, adds sleep safety checks, DBcore checks for leaks (#38363)

* Qdels all queries, adds sleep handling

* DB Core messages admins about undeleted queries

* Compile fixes. Adds missing set waitfor

* Remove world/New shennanigans. Add DBQuery/BlockingExecute()

* Less spammy notifications to admins about undeleted queries

* Increase dbcore fire time to 1 minute

* Upgrade undeleted query warning

* Better place of death

* Fix build

* Remove BlockingExecute, see BSQL PR for why

* Yep, missed that one.

* Psyche, that's the WRONG QUERY!!

code/controllers/subsystem/blackbox.dm

@@ -24,6 +24,17 @@ SUBSYSTEM_DEF(blackbox)
 
 //poll population
 /datum/controller/subsystem/blackbox/fire()
+	set waitfor = FALSE	//for population query
+
+	CheckPlayerCount()
+
+	if(CONFIG_GET(flag/use_exp_tracking))
+		if((triggertime < 0) || (world.time > (triggertime +3000)))	//subsystem fires once at roundstart then once every 10 minutes. a 5 min check skips the first fire. The <0 is midnight rollover check
+			update_exp(10,FALSE)
+
+/datum/controller/subsystem/blackbox/proc/CheckPlayerCount()
+	set waitfor = FALSE
+
 	if(!SSdbcore.Connect())
 		return
 	var/playercount = 0
@@ -33,11 +44,7 @@ SUBSYSTEM_DEF(blackbox)
 	var/admincount = GLOB.admins.len
 	var/datum/DBQuery/query_record_playercount = SSdbcore.NewQuery("INSERT INTO [format_table_name("legacy_population")] (playercount, admincount, time, server_ip, server_port, round_id) VALUES ([playercount], [admincount], '[SQLtime()]', INET_ATON(IF('[world.internet_address]' LIKE '', '0', '[world.internet_address]')), '[world.port]', '[GLOB.round_id]')")
 	query_record_playercount.Execute()
-
-	if(CONFIG_GET(flag/use_exp_tracking))
-		if((triggertime < 0) || (world.time > (triggertime +3000)))	//subsystem fires once at roundstart then once every 10 minutes. a 5 min check skips the first fire. The <0 is midnight rollover check
-			update_exp(10,FALSE)
-
+	qdel(query_record_playercount)
 
 /datum/controller/subsystem/blackbox/Recover()
 	feedback = SSblackbox.feedback
@@ -255,6 +262,7 @@ Versioning
 	key_type = new_key_type
 
 /datum/controller/subsystem/blackbox/proc/ReportDeath(mob/living/L)
+	set waitfor = FALSE
 	if(sealed)
 		return
 	if(!L || !L.key || !L.mind)
@@ -267,28 +275,50 @@ Versioning
 		first_death["area"] = "[AREACOORD(L)]"
 		first_death["damage"] = "<font color='#FF5555'>[L.getBruteLoss()]</font>/<font color='orange'>[L.getFireLoss()]</font>/<font color='lightgreen'>[L.getToxLoss()]</font>/<font color='lightblue'>[L.getOxyLoss()]</font>/<font color='pink'>[L.getCloneLoss()]</font>"
 		first_death["last_words"] = L.last_words
+	var/sqlname = L.real_name
+	var/sqlkey = L.ckey
+	var/sqljob = L.mind.assigned_role
+	var/sqlspecial = L.mind.special_role
+	var/sqlpod = get_area_name(L, TRUE)
+	var/laname = L.lastattacker
+	var/lakey = L.lastattackerckey
+	var/sqlbrute = L.getBruteLoss()
+	var/sqlfire = L.getFireLoss()
+	var/sqlbrain = L.getBrainLoss()
+	var/sqloxy = L.getOxyLoss()
+	var/sqltox = L.getToxLoss()
+	var/sqlclone = L.getCloneLoss()
+	var/sqlstamina = L.getStaminaLoss()
+	var/x_coord = L.x
+	var/y_coord = L.y
+	var/z_coord = L.z
+	var/last_words = L.last_words
+	var/suicide = L.suiciding
+	var/map = SSmapping.config.map_name
+
 	if(!SSdbcore.Connect())
 		return
-	var/area/placeofdeath = get_area(L)
-	var/sqlname = sanitizeSQL(L.real_name)
-	var/sqlkey = sanitizeSQL(L.ckey)
-	var/sqljob = sanitizeSQL(L.mind.assigned_role)
-	var/sqlspecial = sanitizeSQL(L.mind.special_role)
-	var/sqlpod = sanitizeSQL(placeofdeath.name)
-	var/laname = sanitizeSQL(L.lastattacker)
-	var/lakey = sanitizeSQL(L.lastattackerckey)
-	var/sqlbrute = sanitizeSQL(L.getBruteLoss())
-	var/sqlfire = sanitizeSQL(L.getFireLoss())
-	var/sqlbrain = sanitizeSQL(L.getBrainLoss())
-	var/sqloxy = sanitizeSQL(L.getOxyLoss())
-	var/sqltox = sanitizeSQL(L.getToxLoss())
-	var/sqlclone = sanitizeSQL(L.getCloneLoss())
-	var/sqlstamina = sanitizeSQL(L.getStaminaLoss())
-	var/x_coord = sanitizeSQL(L.x)
-	var/y_coord = sanitizeSQL(L.y)
-	var/z_coord = sanitizeSQL(L.z)
-	var/last_words = sanitizeSQL(L.last_words)
-	var/suicide = sanitizeSQL(L.suiciding)
-	var/map = sanitizeSQL(SSmapping.config.map_name)
+	
+	sqlname = sanitizeSQL(sqlname)
+	sqlkey = sanitizeSQL(sqlkey)
+	sqljob = sanitizeSQL(sqljob)
+	sqlspecial = sanitizeSQL(sqlspecial)
+	sqlpod = sanitizeSQL(sqlpod)
+	laname = sanitizeSQL(laname)
+	lakey = sanitizeSQL(lakey)
+	sqlbrute = sanitizeSQL(sqlbrute)
+	sqlfire = sanitizeSQL(sqlfire)
+	sqlbrain = sanitizeSQL(sqlbrain)
+	sqloxy = sanitizeSQL(sqloxy)
+	sqltox = sanitizeSQL(sqltox)
+	sqlclone = sanitizeSQL(sqlclone)
+	sqlstamina = sanitizeSQL(sqlstamina)
+	x_coord = sanitizeSQL(x_coord)
+	y_coord = sanitizeSQL(y_coord)
+	z_coord = sanitizeSQL(z_coord)
+	last_words = sanitizeSQL(last_words)
+	suicide = sanitizeSQL(suicide)
+	map = sanitizeSQL(map)
 	var/datum/DBQuery/query_report_death = SSdbcore.NewQuery("INSERT INTO [format_table_name("death")] (pod, x_coord, y_coord, z_coord, mapname, server_ip, server_port, round_id, tod, job, special, name, byondkey, laname, lakey, bruteloss, fireloss, brainloss, oxyloss, toxloss, cloneloss, staminaloss, last_words, suicide) VALUES ('[sqlpod]', '[x_coord]', '[y_coord]', '[z_coord]', '[map]', INET_ATON(IF('[world.internet_address]' LIKE '', '0', '[world.internet_address]')), '[world.port]', [GLOB.round_id], '[SQLtime()]', '[sqljob]', '[sqlspecial]', '[sqlname]', '[sqlkey]', '[laname]', '[lakey]', [sqlbrute], [sqlfire], [sqlbrain], [sqloxy], [sqltox], [sqlclone], [sqlstamina], '[last_words]', [suicide])")
 	query_report_death.Execute()
+	qdel(query_report_death)

code/controllers/subsystem/dbcore.dm

@@ -1,6 +1,7 @@
 SUBSYSTEM_DEF(dbcore)
 	name = "Database"
-	flags = SS_NO_FIRE
+	flags = SS_BACKGROUND
+	wait = 1 MINUTES
 	init_order = INIT_ORDER_DBCORE
 	var/const/FAILED_DB_CONNECTION_CUTOFF = 5
 
@@ -25,6 +26,8 @@ SUBSYSTEM_DEF(dbcore)
 	var/_db_con// This variable contains a reference to the actual database connection.
 	var/failed_connections = 0
 
+	var/list/active_queries = list()
+
 /datum/controller/subsystem/dbcore/PreInit()
 	if(!_db_con)
 		_db_con = _dm_db_new_con()
@@ -40,6 +43,15 @@ SUBSYSTEM_DEF(dbcore)
 
 	return ..()
 
+/datum/controller/subsystem/dbcore/fire()
+	for(var/I in active_queries)
+		var/datum/DBQuery/Q = I
+		if(world.time - Q.last_activity_time > (5 MINUTES))
+			message_admins("Found undeleted query, please check the server logs and notify coders.")
+			log_sql("Undeleted query: \"[Q.sql]\" LA: [Q.last_activity] LAT: [Q.last_activity_time]")
+			qdel(Q)
+		if(MC_TICK_CHECK)
+			return
 
 /datum/controller/subsystem/dbcore/Recover()
 	_db_con = SSdbcore._db_con
@@ -49,12 +61,13 @@ SUBSYSTEM_DEF(dbcore)
 	if(SSdbcore.Connect())
 		var/datum/DBQuery/query_round_shutdown = SSdbcore.NewQuery("UPDATE [format_table_name("round")] SET shutdown_datetime = Now(), end_state = '[sanitizeSQL(SSticker.end_state)]' WHERE id = [GLOB.round_id]")
 		query_round_shutdown.Execute()
+		qdel(query_round_shutdown)
 	if(IsConnected())
 		Disconnect()
 
 //nu
 /datum/controller/subsystem/dbcore/can_vv_get(var_name)
-	return var_name != "_db_con" && ..()
+	return var_name != NAMEOF(src, _db_con) && var_name != NAMEOF(src, active_queries) && ..()
 
 /datum/controller/subsystem/dbcore/vv_edit_var(var_name, var_value)
 	if(var_name == "_db_con")
@@ -98,33 +111,38 @@ SUBSYSTEM_DEF(dbcore)
 			else
 				schema_mismatch = 2 //flag admin message about no schema version
 				log_sql("Could not get schema version from database")
+			qdel(query_db_version)
 		else
 			log_sql("Your server failed to establish a connection with the database.")
 	else
 		log_sql("Database is not enabled in configuration.")
 
 /datum/controller/subsystem/dbcore/proc/SetRoundID()
-	if(CONFIG_GET(flag/sql_enabled))
-		if(SSdbcore.Connect())
-			var/datum/DBQuery/query_round_initialize = SSdbcore.NewQuery("INSERT INTO [format_table_name("round")] (initialize_datetime, server_ip, server_port) VALUES (Now(), INET_ATON(IF('[world.internet_address]' LIKE '', '0', '[world.internet_address]')), '[world.port]')")
-			query_round_initialize.Execute()
-			var/datum/DBQuery/query_round_last_id = SSdbcore.NewQuery("SELECT LAST_INSERT_ID()")
-			query_round_last_id.Execute()
-			if(query_round_last_id.NextRow())
-				GLOB.round_id = query_round_last_id.item[1]
+	if(!Connect())
+		return
+	var/datum/DBQuery/query_round_initialize = SSdbcore.NewQuery("INSERT INTO [format_table_name("round")] (initialize_datetime, server_ip, server_port) VALUES (Now(), INET_ATON(IF('[world.internet_address]' LIKE '', '0', '[world.internet_address]')), '[world.port]')")
+	query_round_initialize.Execute()
+	qdel(query_round_initialize)
+	var/datum/DBQuery/query_round_last_id = SSdbcore.NewQuery("SELECT LAST_INSERT_ID()")
+	query_round_last_id.Execute()
+	if(query_round_last_id.NextRow())
+		GLOB.round_id = query_round_last_id.item[1]
+	qdel(query_round_last_id)
 
 /datum/controller/subsystem/dbcore/proc/SetRoundStart()
-	if(CONFIG_GET(flag/sql_enabled))
-		if(SSdbcore.Connect())
-			var/datum/DBQuery/query_round_start = SSdbcore.NewQuery("UPDATE [format_table_name("round")] SET start_datetime = Now() WHERE id = [GLOB.round_id]")
-			query_round_start.Execute()
+	if(!Connect())
+		return
+	var/datum/DBQuery/query_round_start = SSdbcore.NewQuery("UPDATE [format_table_name("round")] SET start_datetime = Now() WHERE id = [GLOB.round_id]")
+	query_round_start.Execute()
+	qdel(query_round_start)
 
 /datum/controller/subsystem/dbcore/proc/SetRoundEnd()
-	if(CONFIG_GET(flag/sql_enabled))
-		if(SSdbcore.Connect())
-			var/sql_station_name = sanitizeSQL(station_name())
-			var/datum/DBQuery/query_round_end = SSdbcore.NewQuery("UPDATE [format_table_name("round")] SET end_datetime = Now(), game_mode_result = '[sanitizeSQL(SSticker.mode_result)]', station_name = '[sql_station_name]' WHERE id = [GLOB.round_id]")
-			query_round_end.Execute()
+	if(!Connect())
+		return
+	var/sql_station_name = sanitizeSQL(station_name())
+	var/datum/DBQuery/query_round_end = SSdbcore.NewQuery("UPDATE [format_table_name("round")] SET end_datetime = Now(), game_mode_result = '[sanitizeSQL(SSticker.mode_result)]', station_name = '[sql_station_name]' WHERE id = [GLOB.round_id]")
+	query_round_end.Execute()
+	qdel(query_round_end)
 
 /datum/controller/subsystem/dbcore/proc/Disconnect()
 	failed_connections = 0
@@ -215,6 +233,7 @@ Delayed insert mode was removed in mysql 7 and only works with MyISAM type table
 		return Query.warn_execute()
 	else
 		return Query.Execute()
+	qdel(Query)
 
 
 /datum/DBQuery
@@ -223,11 +242,14 @@ Delayed insert mode was removed in mysql 7 and only works with MyISAM type table
 	var/list/columns //list of DB Columns populated by Columns()
 	var/list/conversions
 	var/list/item  //list of data values populated by NextRow()
-
+	var/last_activity
+	var/last_activity_time
 	var/datum/controller/subsystem/dbcore/db_connection
 	var/_db_query
 
 /datum/DBQuery/New(sql_query, datum/controller/subsystem/dbcore/connection_handler, cursor_handler)
+	SSdbcore.active_queries[src] = TRUE
+	Activity("Created")
 	if(sql_query)
 		sql = sql_query
 	if(connection_handler)
@@ -237,12 +259,22 @@ Delayed insert mode was removed in mysql 7 and only works with MyISAM type table
 	item = list()
 	_db_query = _dm_db_new_query()
 
+/datum/DBQuery/Destroy()
+	Close()
+	SSdbcore.active_queries -= src
+	return ..()
+
+/datum/DBQuery/proc/Activity(activity)
+	last_activity = activity
+	last_activity_time = world.time
+
 /datum/DBQuery/proc/warn_execute()
 	. = Execute()
 	if(!.)
 		to_chat(usr, "<span class='danger'>A SQL error occurred during this operation, check the server logs.</span>")
 
 /datum/DBQuery/proc/Execute(sql_query = sql, cursor_handler = default_cursor, log_error = TRUE)
+	Activity("Execute")
 	var/start_time
 	var/timeout = CONFIG_GET(number/query_debug_log_timeout)
 	if(timeout)
@@ -263,6 +295,7 @@ Delayed insert mode was removed in mysql 7 and only works with MyISAM type table
 	message_admins("HEY! A database query may have timed out. Did the server just hang? <a href='?_src_=holder;[HrefToken()];slowquery=yes'>\[YES\]</a>|<a href='?_src_=holder;[HrefToken()];slowquery=no'>\[NO\]</a>")
 
 /datum/DBQuery/proc/NextRow()
+	Activity("NextRow")
 	return _dm_db_next_row(_db_query,item,conversions)
 
 /datum/DBQuery/proc/RowsAffected()

code/controllers/subsystem/job.dm

@@ -85,7 +85,7 @@ SUBSYSTEM_DEF(job)
 		var/datum/job/job = GetJob(rank)
 		if(!job)
 			return FALSE
-		if(jobban_isbanned(player, rank))
+		if(jobban_isbanned(player, rank) || QDELETED(player))
 			return FALSE
 		if(!job.player_old_enough(player.client))
 			return FALSE
@@ -107,7 +107,7 @@ SUBSYSTEM_DEF(job)
 	Debug("Running FOC, Job: [job], Level: [level], Flag: [flag]")
 	var/list/candidates = list()
 	for(var/mob/dead/new_player/player in unassigned)
-		if(jobban_isbanned(player, job.title))
+		if(jobban_isbanned(player, job.title) || QDELETED(player))
 			Debug("FOC isbanned failed, Player: [player]")
 			continue
 		if(!job.player_old_enough(player.client))
@@ -140,7 +140,10 @@ SUBSYSTEM_DEF(job)
 		if(job.title in GLOB.command_positions) //If you want a command position, select it!
 			continue
 
-		if(jobban_isbanned(player, job.title))
+		if(jobban_isbanned(player, job.title) || QDELETED(player))
+			if(QDELETED(player))
+				Debug("GRJ isbanned failed, Player deleted")
+				break
 			Debug("GRJ isbanned failed, Player: [player], Job: [job.title]")
 			continue
 
@@ -316,6 +319,10 @@ SUBSYSTEM_DEF(job)
 					Debug("DO isbanned failed, Player: [player], Job:[job.title]")
 					continue
 
+				if(QDELETED(player))
+					Debug("DO player deleted during job ban check")
+					break
+
 				if(!job.player_old_enough(player.client))
 					Debug("DO player not old enough, Player: [player], Job:[job.title]")
 					continue
@@ -344,7 +351,7 @@ SUBSYSTEM_DEF(job)
 	for(var/mob/dead/new_player/player in unassigned)
 		if(PopcapReached())
 			RejectPlayer(player)
-		else if(jobban_isbanned(player, SSjob.overflow_role))
+		else if(jobban_isbanned(player, SSjob.overflow_role) || QDELETED(player))
 			GiveRandomJob(player) //you get to roll for random before everyone else just to be sure you don't get overflow. you're so speshul
 
 	for(var/mob/dead/new_player/player in unassigned)
@@ -482,7 +489,7 @@ SUBSYSTEM_DEF(job)
 		for(var/mob/dead/new_player/player in GLOB.player_list)
 			if(!(player.ready == PLAYER_READY_TO_PLAY && player.mind && !player.mind.assigned_role))
 				continue //This player is not ready
-			if(jobban_isbanned(player, job.title))
+			if(jobban_isbanned(player, job.title) || QDELETED(player))
 				banned++
 				continue
 			if(!job.player_old_enough(player.client))

code/controllers/subsystem/mapping.dm

@@ -219,6 +219,7 @@ SUBSYSTEM_DEF(mapping)
 	if(SSdbcore.Connect())
 		var/datum/DBQuery/query_round_map_name = SSdbcore.NewQuery("UPDATE [format_table_name("round")] SET map_name = '[config.map_name]' WHERE id = [GLOB.round_id]")
 		query_round_map_name.Execute()
+		qdel(query_round_map_name)
 
 #ifndef LOWMEMORYMODE
 	// TODO: remove this when the DB is prepared for the z-levels getting reordered