branches/5.1:

Fix Bug#40760 "set global innodb_thread_concurrency = 0;" is not safe

The config param innodb_thread_concurrency is dynamically set and is
read when a thread enters/exits innodb. If the value is changed between
the enter and exit time the behaviour becomes erratic.
The fix is not to use srv_thread_concurrency when exiting, instead use
the flag trx->declared_to_be_inside_innodb.

rb://57

Approved by: Marko

handler/ha_innodb.cc

@@ -461,7 +461,7 @@ innodb_srv_conc_exit_innodb(
 /*========================*/
 	trx_t*	trx)	/* in: transaction handle */
 {
-	if (UNIV_LIKELY(!srv_thread_concurrency)) {
+	if (UNIV_LIKELY(!trx->declared_to_be_inside_innodb)) {
 
 		return;
 	}

srv/srv0srv.c

@@ -283,13 +283,16 @@ ulong	srv_commit_concurrency	= 0;
 
 os_fast_mutex_t	srv_conc_mutex;		/* this mutex protects srv_conc data
 					structures */
-lint	srv_conc_n_threads	= 0;	/* number of OS threads currently
-					inside InnoDB; it is not an error
-					if this drops temporarily below zero
-					because we do not demand that every
-					thread increments this, but a thread
-					waiting for a lock decrements this
-					temporarily */
+lint	srv_conc_n_threads	= 0;	/* number of transactions that
+					have declared_to_be_inside_innodb
+					set. It used to be a non-error
+					for this value to drop below
+					zero temporarily. This is no
+					longer true. We'll, however,
+					keep the lint datatype to add
+					assertions to catch any corner
+					cases that we may have
+					missed. */
 ulint	srv_conc_n_waiting_threads = 0;	/* number of OS threads waiting in the
 					FIFO for a permission to enter InnoDB
 					*/
@@ -1020,6 +1023,8 @@ srv_conc_enter_innodb(
 		return;
 	}
 
+	ut_ad(srv_conc_n_threads >= 0);
+
 	if (srv_conc_n_threads < (lint)srv_thread_concurrency) {
 
 		srv_conc_n_threads++;
@@ -1146,6 +1151,8 @@ srv_conc_force_enter_innodb(
 		return;
 	}
 
+	ut_ad(srv_conc_n_threads >= 0);
+
 	os_fast_mutex_lock(&srv_conc_mutex);
 
 	srv_conc_n_threads++;
@@ -1167,11 +1174,6 @@ srv_conc_force_exit_innodb(
 {
 	srv_conc_slot_t*	slot	= NULL;
 
-	if (UNIV_LIKELY(!srv_thread_concurrency)) {
-
-		return;
-	}
-
 	if (trx->mysql_thd != NULL
 	    && thd_is_replication_slave_thread(trx->mysql_thd)) {
 
@@ -1185,6 +1187,7 @@ srv_conc_force_exit_innodb(
 
 	os_fast_mutex_lock(&srv_conc_mutex);
 
+	ut_ad(srv_conc_n_threads > 0);
 	srv_conc_n_threads--;
 	trx->declared_to_be_inside_innodb = FALSE;
 	trx->n_tickets_to_enter_innodb = 0;

trx/trx0trx.c

@@ -287,6 +287,10 @@ trx_free(
 		      "InnoDB: inside InnoDB.\n", stderr);
 		trx_print(stderr, trx, 600);
 		putc('\n', stderr);
+
+		/* This is an error but not a fatal error. We must keep
+		the counters like srv_conc_n_threads accurate. */
+		srv_conc_force_exit_innodb(trx);
 	}
 
 	if (trx->n_mysql_tables_in_use != 0