Merge pull request #4189 from sysown/v2.x-glovars_shutdown

Alternative to Fix invalid memory accesses to 'GloVars::checksums_values' during shutdown

include/ProxySQL_Cluster.hpp

@@ -61,7 +61,6 @@ class ProxySQL_Checksum_Value_2: public ProxySQL_Checksum_Value {
 	time_t last_changed;
 	unsigned int diff_check;
 	ProxySQL_Checksum_Value_2() {
-		ProxySQL_Checksum_Value();
 		last_changed = 0;
 		last_updated = 0;
 		diff_check = 0;

include/proxysql_glovars.hpp

@@ -31,20 +31,29 @@ class ProxySQL_Checksum_Value {
 	char *checksum;
 	unsigned long long version;
 	unsigned long long epoch;
+	bool in_shutdown;
 	ProxySQL_Checksum_Value() {
 		checksum = (char *)malloc(20);
 		memset(checksum,0,20);
 		version = 0;
 		epoch = 0;
+		in_shutdown = false;
 	}
 	void set_checksum(char *c) {
 		memset(checksum,0,20);
 		strncpy(checksum,c,18);
 		replace_checksum_zeros(checksum);
 	}
 	~ProxySQL_Checksum_Value() {
-		free(checksum);
-		checksum = NULL;
+		if (in_shutdown == false) {
+			/**
+			 * @brief the in_shutdown flag is false by default, but set to true
+			 * in the destructor of ProxySQL_GlobalVariables.
+			 * See comments there for futher details.
+			 */
+			free(checksum);
+			checksum = NULL;
+		}
 	}
 };
 
@@ -125,29 +134,18 @@ class ProxySQL_GlobalVariables {
 	} statuses;
 	pthread_mutex_t checksum_mutex;
 	time_t epoch_version;
-	/**
-	 * @brief Anonymous union holding just the member 'checksums_values'.
-	 * @details This encapsulation is performed to prevent the call to 'ProxySQL_Checksum_Value' destructor for
-	 *  'checksums_values' members. These checksums memory is never freed during ProxySQL execution
-	 *  lifetime (only reused during update operations), and they are concurrently access by multiple threads,
-	 *  including during shutdown phase. Since 'GloVars' (unique instance of this class) is declared global,
-	 *  it's impossible to control de destruction order with respect to the other modules. To avoid invalid
-	 *  memory accesses during shutdown, we avoid calling the destructor of the members at all.
-	 */
-	union {
-		struct {
-			ProxySQL_Checksum_Value admin_variables;
-			ProxySQL_Checksum_Value mysql_query_rules;
-			ProxySQL_Checksum_Value mysql_servers;
-			ProxySQL_Checksum_Value mysql_users;
-			ProxySQL_Checksum_Value mysql_variables;
-			ProxySQL_Checksum_Value ldap_variables;
-			ProxySQL_Checksum_Value proxysql_servers;
-			uint64_t global_checksum;
-			unsigned long long updates_cnt;
-			unsigned long long dumped_at;
-		} checksums_values;
-	};
+	struct {
+		ProxySQL_Checksum_Value admin_variables;
+		ProxySQL_Checksum_Value mysql_query_rules;
+		ProxySQL_Checksum_Value mysql_servers;
+		ProxySQL_Checksum_Value mysql_users;
+		ProxySQL_Checksum_Value mysql_variables;
+		ProxySQL_Checksum_Value ldap_variables;
+		ProxySQL_Checksum_Value proxysql_servers;
+		uint64_t global_checksum;
+		unsigned long long updates_cnt;
+		unsigned long long dumped_at;
+	} checksums_values;
 	uint64_t generate_global_checksum();
 	ProxySQL_GlobalVariables();
 	~ProxySQL_GlobalVariables();

lib/ProxySQL_GloVars.cpp

@@ -77,15 +77,26 @@ ProxySQL_GlobalVariables::~ProxySQL_GlobalVariables() {
 		free(ldap_auth_plugin);
 		ldap_auth_plugin = NULL;
 	}
+	/**
+	 * @brief set in_shutdown flag just the member 'checksums_values'.
+	 * @details This is performed to prevent the free() inside the 'ProxySQL_Checksum_Value' destructor for
+	 *  'checksums_values' members. These checksums memory is never freed during ProxySQL execution
+	 *  lifetime (only reused during update operations), and they are concurrently access by multiple threads,
+	 *  including during shutdown phase. Since 'GloVars' (unique instance of this class) is declared global,
+	 *  it's impossible to control de destruction order with respect to the other modules. To avoid invalid
+	 *  memory accesses during shutdown, we avoid calling free() inside the destructor of the members.
+	 */
+	checksums_values.admin_variables.in_shutdown = true;
+	checksums_values.mysql_query_rules.in_shutdown = true;
+	checksums_values.mysql_servers.in_shutdown = true;
+	checksums_values.mysql_users.in_shutdown = true;
+	checksums_values.mysql_variables.in_shutdown = true;
+	checksums_values.ldap_variables.in_shutdown = true;
+	checksums_values.proxysql_servers.in_shutdown = true;
 };
 
-/**
- * @brief ProxySQL_GlobalVariables constructor
- * @details 'checksums_values' constructor is required to be explicitly called as it's encapsulated in a
- *  anonymous union.
- */
 ProxySQL_GlobalVariables::ProxySQL_GlobalVariables() :
-	prometheus_registry(std::make_shared<prometheus::Registry>()), checksums_values()
+	prometheus_registry(std::make_shared<prometheus::Registry>())
 {
 	confFile=NULL;
 	__cmd_proxysql_config_file=NULL;