[MDEV-10365] Race condition in error handling of INSERT DELAYED Created: 2016-07-12  Updated: 2016-08-04  Resolved: 2016-08-04

Status: Closed
Project: MariaDB Server
Component/s: Data Manipulation - Insert
Affects Version/s: 5.5, 10.0, 10.1, 10.2
Fix Version/s: 5.5.51, 10.1.17, 10.0.27, 10.2.2

Type: Bug Priority: Major
Reporter: Sergey Vojtovich Assignee: Sergey Vojtovich
Resolution: Fixed Votes: 0
Labels: None

Sprint: 5.5.51 & 10.2.2

 Description   

This race condition is hard to reproduce with unmodified code. But it is easily reproducible with error simulation as in the following patch:

diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index c60ef6f..d10dc3d 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -3021,7 +3021,7 @@ bool Delayed_insert::handle_inserts(void)
   table->use_all_columns();
 
   thd_proc_info(&thd, "upgrading lock");
-  if (thr_upgrade_write_delay_lock(*thd.lock->locks, delayed_lock,
+  if (1 || thr_upgrade_write_delay_lock(*thd.lock->locks, delayed_lock,
                                    thd.variables.lock_wait_timeout))
   {
     /*

MTR test case:

CREATE TABLE t1(a INT);
let $i= 10000;
disable_query_log;
while ($i)
{
  --error 0,1150
  INSERT DELAYED INTO t1 VALUES(1),(2),(3),(4),(5),(6),(7),(8),(9),(10),(11),(12),(13),(14),(15),(16),(17),(18),(19),(20);
  dec $i;
}
enable_query_log;
DROP TABLE t1;

Running this test makes threads stuck as following:

+------+---------+-----------+------+---------+------+----------------------------+------------------------------------------------------------------------------------------------------+----------+
| Id   | User    | Host      | db   | Command | Time | State                      | Info                                                                                                 | Progress |
+------+---------+-----------+------+---------+------+----------------------------+------------------------------------------------------------------------------------------------------+----------+
|    2 | root    | localhost | test | Query   |  261 | waiting for handler insert | INSERT DELAYED INTO t1 VALUES(1),(2),(3),(4),(5),(6),(7),(8),(9),(10),(11),(12),(13),(14),(15),(16), |    0.000 |
| 5026 | root    | localhost | NULL | Query   |    0 | NULL                       | show processlist                                                                                     |    0.000 |
| 8018 | DELAYED | localhost | test | Killed  |  263 | upgrading lock             |                                                                                                      |    0.000 |
| 8025 | DELAYED | localhost | test | Killed  |  263 | upgrading lock             |                                                                                                      |    0.000 |
| 8028 | DELAYED | localhost | test | Killed  |  263 | upgrading lock             |                                                                                                      |    0.000 |
| 8029 | DELAYED | localhost | test | Killed  |  263 | upgrading lock             |                                                                                                      |    0.000 |
| 8043 | DELAYED | localhost | test | Killed  |  263 | upgrading lock             |                                                                                                      |    0.000 |
| 8056 | DELAYED | localhost | test | Killed  |  263 | upgrading lock             |                                                                                                      |    0.000 |
| 8150 | DELAYED | localhost | test | Killed  |  263 | upgrading lock             |                                                                                                      |    0.000 |
| 8170 | DELAYED | localhost | test | Killed  |  262 | upgrading lock             |                                                                                                      |    0.000 |
| 8207 | DELAYED | localhost | test | Killed  |  262 | upgrading lock             |                                                                                                      |    0.000 |
| 8243 | DELAYED | localhost | test | Killed  |  262 | upgrading lock             |                                                                                                      |    0.000 |
| 8295 | DELAYED | localhost | test | Killed  |  261 | upgrading lock             |                                                                                                      |    0.000 |
| 8305 | DELAYED | localhost | test | Killed  |  261 | upgrading lock             |                                                                                                      |    0.000 |
+------+---------+-----------+------+---------+------+----------------------------+------------------------------------------------------------------------------------------------------+----------+

The following patch makes it work:

diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc
index c60ef6f..d10dc3d 100644
--- a/sql/sql_insert.cc
+++ b/sql/sql_insert.cc
@@ -3238,6 +3238,7 @@ bool Delayed_insert::handle_inserts(void)
   max_rows= 0;                                  // For DBUG output
 #endif
   /* Remove all not used rows */
+  mysql_mutex_lock(&mutex);
   while ((row=rows.get()))
   {
     if (table->s->blob_fields)
@@ -3254,7 +3255,6 @@ bool Delayed_insert::handle_inserts(void)
   }
   DBUG_PRINT("error", ("dropped %lu rows after an error", max_rows));
   thread_safe_increment(delayed_insert_errors, &LOCK_delayed_status);
-  mysql_mutex_lock(&mutex);
   DBUG_RETURN(1);
 }
 #endif /* EMBEDDED_LIBRARY */



 Comments   
Comment by Sergey Vojtovich [ 2016-08-03 ]

serg, please review fix for this bug.

Generated at Thu Feb 08 07:41:41 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.