diff --git a/mysql-test/suite/heap/blob_const_unlock.inc b/mysql-test/suite/heap/blob_const_unlock.inc new file mode 100644 index 0000000000000..8907d3dfd792c --- /dev/null +++ b/mysql-test/suite/heap/blob_const_unlock.inc @@ -0,0 +1,58 @@ +# +# One round of the const-table blob race. +# +# Expects t1 (the const table), t2 (the table it is joined against), a +# procedure churn_blob_memory() that rewrites t1's blob memory, and +# $reader_id / $writer_id. +# + +--connection default +# --ps-protocol executes every complete SELECT twice and compares the two +# result sets. GET_LOCK() is recursive, so a doubly executed acquisition +# takes the gate twice while the single RELEASE_LOCK() below drops only one +# reference. The gate would then stay shut and the reader would sit there +# until its own timeout expired. +--disable_ps2_protocol # because SELECT with side effects +SELECT GET_LOCK('const_row_gate', 60); +--enable_ps2_protocol + +--connection reader +# t1 holds a single row, so it is read during optimization and its row is +# kept in record[0] for the rest of the statement. GET_LOCK() is evaluated +# per row of t2 during execution, i.e. after the const row is in hand and +# after the const tables have been unlocked, and parks the statement there. +--send SELECT LEFT(t1.b, 16) AS head, LENGTH(t1.b) AS len FROM t1, t2 WHERE t2.a = GET_LOCK('const_row_gate', 60) + +--connection default +let $wait_condition= + SELECT COUNT(*) FROM information_schema.PROCESSLIST + WHERE ID = $reader_id AND STATE = 'User lock'; +--source include/wait_condition.inc + +--connection writer +--send CALL churn_blob_memory() + +--connection default +# The procedure signals before its first statement, so once the signal +# arrives the writer is executing. From there it either blocks on t1's +# surviving read lock or runs to completion; both mean it has had its turn +# and the reader can be let go. +SET DEBUG_SYNC= 'now WAIT_FOR writer_started'; +let $wait_condition= + SELECT COUNT(*) FROM information_schema.PROCESSLIST + WHERE ID = $writer_id + AND (STATE = 'Waiting for table level lock' OR COMMAND = 'Sleep'); +--source include/wait_condition.inc + +DO RELEASE_LOCK('const_row_gate'); + +--connection reader +--reap +# The reader took the gate over when it was released. +DO RELEASE_ALL_LOCKS(); + +--connection writer +--reap + +--connection default +SET DEBUG_SYNC= 'RESET'; diff --git a/mysql-test/suite/heap/blob_const_unlock.result b/mysql-test/suite/heap/blob_const_unlock.result new file mode 100644 index 0000000000000..ef5c0dc759857 --- /dev/null +++ b/mysql-test/suite/heap/blob_const_unlock.result @@ -0,0 +1,78 @@ +CREATE TABLE t1 (a INT, b BLOB) ENGINE=MEMORY; +INSERT INTO t1 VALUES (1, REPEAT('x', 4000)); +CREATE TABLE t2 (a INT) ENGINE=MEMORY; +INSERT INTO t2 VALUES (1),(2),(3),(4); +connect reader,localhost,root,,test; +connect writer,localhost,root,,test; +connection reader; +connection writer; +SET SESSION lock_wait_timeout= 60; +# +# The freed blob blocks are handed to another row +# +connection default; +CREATE PROCEDURE churn_blob_memory() +BEGIN +SET DEBUG_SYNC= 'now SIGNAL writer_started'; +UPDATE t1 SET b= REPEAT('y', 4000) WHERE a = 1; +INSERT INTO t1 VALUES (2, REPEAT('z', 4000)), (3, REPEAT('w', 4000)); +END| +connection default; +SELECT GET_LOCK('const_row_gate', 60); +GET_LOCK('const_row_gate', 60) +1 +connection reader; +SELECT LEFT(t1.b, 16) AS head, LENGTH(t1.b) AS len FROM t1, t2 WHERE t2.a = GET_LOCK('const_row_gate', 60); +connection default; +connection writer; +CALL churn_blob_memory(); +connection default; +SET DEBUG_SYNC= 'now WAIT_FOR writer_started'; +DO RELEASE_LOCK('const_row_gate'); +connection reader; +head len +xxxxxxxxxxxxxxxx 4000 +DO RELEASE_ALL_LOCKS(); +connection writer; +connection default; +SET DEBUG_SYNC= 'RESET'; +connection default; +DROP PROCEDURE churn_blob_memory; +TRUNCATE TABLE t1; +INSERT INTO t1 VALUES (1, REPEAT('x', 4000)); +# +# The blob blocks are released outright +# +CREATE PROCEDURE churn_blob_memory() +BEGIN +SET DEBUG_SYNC= 'now SIGNAL writer_started'; +DELETE FROM t1; +INSERT INTO t1 VALUES (2, REPEAT('z', 9000)); +END| +connection default; +SELECT GET_LOCK('const_row_gate', 60); +GET_LOCK('const_row_gate', 60) +1 +connection reader; +SELECT LEFT(t1.b, 16) AS head, LENGTH(t1.b) AS len FROM t1, t2 WHERE t2.a = GET_LOCK('const_row_gate', 60); +connection default; +connection writer; +CALL churn_blob_memory(); +connection default; +SET DEBUG_SYNC= 'now WAIT_FOR writer_started'; +DO RELEASE_LOCK('const_row_gate'); +connection reader; +head len +xxxxxxxxxxxxxxxx 4000 +DO RELEASE_ALL_LOCKS(); +connection writer; +connection default; +SET DEBUG_SYNC= 'RESET'; +connection default; +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +disconnect reader; +disconnect writer; +DROP PROCEDURE churn_blob_memory; +DROP TABLE t1, t2; diff --git a/mysql-test/suite/heap/blob_const_unlock.test b/mysql-test/suite/heap/blob_const_unlock.test new file mode 100644 index 0000000000000..a107ec5afd5b9 --- /dev/null +++ b/mysql-test/suite/heap/blob_const_unlock.test @@ -0,0 +1,86 @@ +# +# A const table's blob must not outlive the lock that protected it. +# +# A single-row table is read during optimization and its row is kept in +# record[0] for the rest of the statement. The optimizer then releases the +# lock on every const table, on the premise -- stated in its own comment -- +# that all it did was read, so the row is already in hand. +# +# For a MEMORY table with a blob the row is not in hand. hp_read_blobs() +# aims record[0]'s blob pointer straight into the shared record memory +# instead of copying the value, so once the lock is gone another connection +# is free to overwrite, free or recycle the bytes the const row still points +# at, and the statement goes on reading them. +# +# The two rounds below cover the two ways that memory changes hands: reuse +# (UPDATE frees the chain, INSERT hands the same blocks to another row) and +# outright release (DELETE with no WHERE goes through delete_all_rows()). +# +# The reader is parked with GET_LOCK() rather than with a stored function. +# A stored function would put the statement into prelocked mode, and the +# const-table unlock is skipped altogether in that mode, so the code path +# under test would never run. +# +--source include/have_debug_sync.inc + +CREATE TABLE t1 (a INT, b BLOB) ENGINE=MEMORY; +INSERT INTO t1 VALUES (1, REPEAT('x', 4000)); + +# Several rows, so this one cannot become a const table itself and the join +# still has an execution phase for the reader to park in. +CREATE TABLE t2 (a INT) ENGINE=MEMORY; +INSERT INTO t2 VALUES (1),(2),(3),(4); + +connect (reader,localhost,root,,test); +connect (writer,localhost,root,,test); + +--connection reader +let $reader_id= `SELECT CONNECTION_ID()`; + +--connection writer +let $writer_id= `SELECT CONNECTION_ID()`; +# A bad interleaving should fail fast rather than hang for a day. +SET SESSION lock_wait_timeout= 60; + +--echo # +--echo # The freed blob blocks are handed to another row +--echo # +--connection default +DELIMITER |; +CREATE PROCEDURE churn_blob_memory() +BEGIN + SET DEBUG_SYNC= 'now SIGNAL writer_started'; + UPDATE t1 SET b= REPEAT('y', 4000) WHERE a = 1; + INSERT INTO t1 VALUES (2, REPEAT('z', 4000)), (3, REPEAT('w', 4000)); +END| +DELIMITER ;| + +--source blob_const_unlock.inc + +--connection default +DROP PROCEDURE churn_blob_memory; +TRUNCATE TABLE t1; +INSERT INTO t1 VALUES (1, REPEAT('x', 4000)); + +--echo # +--echo # The blob blocks are released outright +--echo # +DELIMITER |; +CREATE PROCEDURE churn_blob_memory() +BEGIN + SET DEBUG_SYNC= 'now SIGNAL writer_started'; + DELETE FROM t1; + INSERT INTO t1 VALUES (2, REPEAT('z', 9000)); +END| +DELIMITER ;| + +--source blob_const_unlock.inc + +--connection default +CHECK TABLE t1; + +disconnect reader; +disconnect writer; + +DROP PROCEDURE churn_blob_memory; +DROP TABLE t1, t2; diff --git a/sql/lock.cc b/sql/lock.cc index 79a3f98f5dda6..32b3e3bde7f87 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -800,6 +800,26 @@ static int unlock_external(THD *thd, TABLE **table,uint count) } +/* + Does this table answer a read with pointers into memory it shares with + other connections, rather than with a copy of the row? + + MEMORY does, for a blob: hp_read_blobs() points record[0] at the blob data + inside HP_SHARE instead of copying it, so the row is only valid while the + lock is held -- once it is gone another connection may free or reuse that + memory. + + A caller unlocking for good does not care. One that unlocks early while + still holding a row does, and passes GET_LOCK_SKIP_ZERO_COPY_ROWS to leave + such a table locked for as long as it means to keep reading the row. +*/ + +static inline bool table_has_zero_copy_rows(const TABLE *t) +{ + return t->s->db_type() == heap_hton && t->s->blob_fields != 0; +} + + /** Get lock structures from table structs and initialize locks. @@ -809,6 +829,8 @@ static int unlock_external(THD *thd, TABLE **table,uint count) - GET_LOCK_UNLOCK : If we should send TL_IGNORE to store lock - GET_LOCK_STORE_LOCKS : Store lock info in TABLE - GET_LOCK_SKIP_SEQUENCES : Ignore sequences (for temporary unlock) + - GET_LOCK_SKIP_ZERO_COPY_ROWS : Ignore tables whose already-read + row would not survive the unlock (for temporary unlock) - GET_LOCK_ON_THD : Store lock in thd->mem_root Temporary tables are not locked (as these are single user), except for @@ -831,7 +853,9 @@ MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count, uint flags) if ((likely(!t->s->tmp_table) || (t->s->tmp_table == TRANSACTIONAL_TMP_TABLE)) && - (!(flags & GET_LOCK_SKIP_SEQUENCES) || t->s->sequence == 0)) + (!(flags & GET_LOCK_SKIP_SEQUENCES) || t->s->sequence == 0) && + (!(flags & GET_LOCK_SKIP_ZERO_COPY_ROWS) || + !table_has_zero_copy_rows(t))) { lock_count+= t->file->lock_count(); table_count++; @@ -864,7 +888,9 @@ MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count, uint flags) THR_LOCK_DATA **locks_start; if ((table->s->tmp_table && table->s->tmp_table != TRANSACTIONAL_TMP_TABLE) - || (flags & GET_LOCK_SKIP_SEQUENCES && table->s->sequence != NULL)) + || (flags & GET_LOCK_SKIP_SEQUENCES && table->s->sequence != NULL) + || (flags & GET_LOCK_SKIP_ZERO_COPY_ROWS && + table_has_zero_copy_rows(table))) continue; lock_type= table->reginfo.lock_type; DBUG_ASSERT(lock_type != TL_WRITE_DEFAULT && lock_type != TL_READ_DEFAULT); diff --git a/sql/lock.h b/sql/lock.h index cfce47ccd15a8..cc8056c786adf 100644 --- a/sql/lock.h +++ b/sql/lock.h @@ -47,6 +47,7 @@ bool lock_object_name(THD *thd, MDL_key::enum_mdl_namespace mdl_type, #define GET_LOCK_ACTION_MASK 1 #define GET_LOCK_ON_THD (1 << 1) #define GET_LOCK_SKIP_SEQUENCES (1 << 2) +#define GET_LOCK_SKIP_ZERO_COPY_ROWS (1 << 3) MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count, uint flags); void reset_lock_data(MYSQL_LOCK *sql_lock, bool unlock); diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 0badf39599cda..a9253bf92aaf6 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -2881,9 +2881,15 @@ int JOIN::optimize_stage2() Unlock all tables, except sequences, as accessing these may still require table updates. It's safe to ignore result code as all tables where opened for read only. + + A const table's row stays in record[0] and is read from there for the + rest of the statement, so a table that answered the read with pointers + into memory it shares with other connections has to stay locked too -- + see table_has_zero_copy_rows(). */ (void) mysql_unlock_some_tables(thd, table, const_tables, - GET_LOCK_SKIP_SEQUENCES); + GET_LOCK_SKIP_SEQUENCES | + GET_LOCK_SKIP_ZERO_COPY_ROWS); } if (!conds && outer_join) {