From: Jon Olav Hauglid
Date: October 6 2010 7:56am
Subject: bzr commit into mysql-5.5-runtime branch (jon.hauglid:3153) Bug#57002
List-Archive: http://lists.mysql.com/commits/120057
X-Bug: 57002
Message-Id: <201010060757.o963Hccm025113@rcsinet15.oracle.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============7188243295943591729=="

--===============7188243295943591729==
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

#At file:///export/home/x/mysql-5.5-runtime-bug57002/ based on revid:jon.hauglid@stripped

 3153 Jon Olav Hauglid	2010-10-06
      Bug #57002 Assert in upgrade_shared_lock_to_exclusive()
                 for ALTER TABLE + MERGE tables
      
      The patch for Bug#56292 changed how metadata locks are taken for MERGE
      tables. After the patch, locking the MERGE table will also lock the
      children tables with the same metadata lock type. This means that 
      LOCK TABLES on a MERGE table also will implicitly do LOCK TABLES on
      the children tables.
      
      A consequence of this change, is that it is possible to do LOCK TABLES
      on a child table both explicitly and implicitly with the same statement
      and that these two locks can be of different strength. For example,
      LOCK TABLES child READ, merge WRITE.
      
      In LOCK TABLES mode, we are not allowed to take new locks and each
      statement must therefore try to find an existing TABLE instance with
      a suitable lock. The code that searched for a suitable TABLE instance,
      only considered table level locks. If a child table was locked twice,
      it was therefore possible for this code to find a TABLE instance with
      suitable table level locks but without suitable metadata lock.
      
      This problem caused the assert in upgrade_shared_lock_to_exclusive()
      to be triggered as it tried to upgrade a MDL_SHARED lock to
      EXCLUSIVE. The problem was a regression caused by the patch for
      Bug#56292.
      
      This patch fixes the problem by partially reverting the changes
      done by Bug#56292. Now, the children tables will only use the
      same metadata lock as the MERGE table for MDL_SHARED_NO_WRITE
      when not in locked tables mode. This means that LOCK TABLE
      on a MERGE table will not implicitly lock the children tables.
      This still fixes the original problem in Bug#56292 without
      causing a regression.
      
      Test case added to merge.test.

    modified:
      mysql-test/r/merge.result
      mysql-test/t/merge.test
      storage/myisammrg/ha_myisammrg.cc
=== modified file 'mysql-test/r/merge.result'
--- a/mysql-test/r/merge.result	2010-09-30 10:43:43 +0000
+++ b/mysql-test/r/merge.result	2010-10-06 07:56:29 +0000
@@ -3486,12 +3486,13 @@ ALTER TABLE m1 ADD INDEX (c1);
 UNLOCK TABLES;
 DROP TABLE m1, t1;
 #
-# Locking the merge table will implicitly lock children.
+# Locking the merge table won't implicitly lock children.
 #
 CREATE TABLE t1 (c1 INT);
 CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1);
 LOCK TABLE m1 WRITE;
 ALTER TABLE t1 ADD INDEX (c1);
+ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
 LOCK TABLE m1 WRITE, t1 WRITE;
 ALTER TABLE t1 ADD INDEX (c1);
 UNLOCK TABLES;
@@ -3661,4 +3662,16 @@ REPAIR TABLE t2 USE_FRM;
 Table	Op	Msg_type	Msg_text
 test.t2	repair	note	The storage engine for the table doesn't support repair
 DROP TABLE t1, t2;
+#
+# Bug#57002 Assert in upgrade_shared_lock_to_exclusive()
+#           for ALTER TABLE + MERGE tables
+#
+DROP TABLE IF EXISTS t1, m1;
+CREATE TABLE t1(a INT) engine=myisam;
+CREATE TABLE m1(a INT) engine=merge UNION(t1);
+LOCK TABLES t1 READ, m1 WRITE;
+ALTER TABLE t1 engine=myisam;
+ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
+UNLOCK TABLES;
+DROP TABLE m1, t1;
 End of 6.0 tests

=== modified file 'mysql-test/t/merge.test'
--- a/mysql-test/t/merge.test	2010-09-30 10:43:43 +0000
+++ b/mysql-test/t/merge.test	2010-10-06 07:56:29 +0000
@@ -2600,11 +2600,12 @@ UNLOCK TABLES;
 DROP TABLE m1, t1;
 
 --echo #
---echo # Locking the merge table will implicitly lock children.
+--echo # Locking the merge table won't implicitly lock children.
 --echo #
 CREATE TABLE t1 (c1 INT);
 CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1);
 LOCK TABLE m1 WRITE;
+--error ER_TABLE_NOT_LOCKED_FOR_WRITE
 ALTER TABLE t1 ADD INDEX (c1);
 LOCK TABLE m1 WRITE, t1 WRITE;
 ALTER TABLE t1 ADD INDEX (c1);
@@ -2776,6 +2777,27 @@ REPAIR TABLE t2 USE_FRM;
 DROP TABLE t1, t2;
 
 
+--echo #
+--echo # Bug#57002 Assert in upgrade_shared_lock_to_exclusive()
+--echo #           for ALTER TABLE + MERGE tables
+--echo #
+
+--disable_warnings
+DROP TABLE IF EXISTS t1, m1;
+--enable_warnings
+
+CREATE TABLE t1(a INT) engine=myisam;
+CREATE TABLE m1(a INT) engine=merge UNION(t1);
+LOCK TABLES t1 READ, m1 WRITE;
+
+# This caused an assert
+--error ER_TABLE_NOT_LOCKED_FOR_WRITE
+ALTER TABLE t1 engine=myisam;
+
+UNLOCK TABLES;
+DROP TABLE m1, t1;
+
+
 --echo End of 6.0 tests
 
 --disable_result_log

=== modified file 'storage/myisammrg/ha_myisammrg.cc'
--- a/storage/myisammrg/ha_myisammrg.cc	2010-09-08 08:25:37 +0000
+++ b/storage/myisammrg/ha_myisammrg.cc	2010-10-06 07:56:29 +0000
@@ -478,8 +478,31 @@ int ha_myisammrg::add_children_list(void
     /* Set the expected table version, to not cause spurious re-prepare. */
     child_l->set_table_ref_id(mrg_child_def->get_child_table_ref_type(),
                               mrg_child_def->get_child_def_version());
-    /* Use the same metadata lock type for children. */
-    child_l->mdl_request.set_type(parent_l->mdl_request.type);
+    /*
+      For statements which acquire a SNW metadata lock on a parent table and
+      then later try to upgrade it to an X lock (e.g. ALTER TABLE), SNW
+      locks should be also taken on the children tables.
+
+      Otherwise we end up in a situation where the thread trying to upgrade SNW
+      to X lock on the parent also holds a SR metadata lock and a read
+      thr_lock.c lock on the child. As a result, another thread might be
+      blocked on the thr_lock.c lock for the child after successfully acquiring
+      a SR or SW metadata lock on it. If at the same time this second thread
+      has a shared metadata lock on the parent table or there is some other
+      thread which has a shared metadata lock on the parent and is waiting for
+      this second thread, we get a deadlock. This deadlock cannot be properly
+      detected by the MDL subsystem as part of the waiting happens within
+      thr_lock.c. By taking SNW locks on the child tables we ensure that any
+      thread which waits for a thread doing SNW -> X upgrade, does this within
+      the MDL subsystem and thus potential deadlocks are exposed to the deadlock
+      detector.
+
+      We don't do the same thing for SNRW locks as this would allow
+      DDL on implicitly locked underlying tables of a MERGE table.
+    */
+    if (! thd->locked_tables_mode &&
+        parent_l->mdl_request.type == MDL_SHARED_NO_WRITE)
+      child_l->mdl_request.set_type(MDL_SHARED_NO_WRITE);
     /* Link TABLE_LIST object into the children list. */
     if (this->children_last_l)
       child_l->prev_global= this->children_last_l;


--===============7188243295943591729==
MIME-Version: 1.0
Content-Type: text/bzr-bundle; charset="us-ascii";
	name="bzr/jon.hauglid@stripped"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: jon.hauglid@stripped
# target_branch: file:///export/home/x/mysql-5.5-runtime-bug57002/
# testament_sha1: 340a18bcb5997d65a37c47dc1d4ee95a9e239b8c
# timestamp: 2010-10-06 09:56:33 +0200
# source_branch: file:///export/home/x/mysql-5.5-bugfixing/
# base_revision_id: jon.hauglid@stripped\
#   mp6vfiu3rl9moqz6
# 
# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWeehpR4ABP9fgFSQef///3//
3+C////wYAzNPrHTvZPe91qVU61uOc5uZ2XXvauh7sqho01QBneEkiaRkaBlU/yNNMhNU/TTSj9U
fqm9KbRqDYp6gMgaaCSgU9T0DTQVN6KYTINAaAAAAABoBqYBUzUyeqnqD0yajIaB6mTQADQaHqAA
AkRCmSMQZBqG1NPUBo00AyDQ0AAGgOMjTJiaDJkwmmQMhoDQGmTQwAmgMJEhNARpoJk0EwCSeUZl
PU9Ro0NNAADTSBZG4BMrZxsDv5YZh8dmraQ4N7rxEfx5/H+2NuGTdr+Nucbdu9/frn5fwf7IzZep
GzdPXswCmqnqLQQD7q6FEFlUOUkjiDaYdmuIeFbT+/G9CalwsMeVcm5WtPetiy8t1/Neq83yY8pC
zQ6kQqGBClAWczotlXVdPKre1e9UphmTabWf+Euh7KXPdOUqOfOKDOl+rmzm+p0GS6rpBdRsVyjT
snVWQiPaO9D/tB8AiKrcDSkxHg72c8Y7lJKDSLRwTvh6L7bU2hmewUMDBPQDyjKMxUimUYW7qoOD
shNmdm55ovMKiZxO5kgSPygqoPsUuiP1oNXKGhKthEKdxBnKXCEJZIY9ccJaQQpIJuTBEbHDB8uO
ORjOQzYMbjhi4Kix2TB8HSnSDLmwjKMrlU4y5Xo3Uy6zomEgx9+bygsw4TBKYxVkDxLEL/C/Ha4z
YYgEIBXB984GYGLj1Tbjo4riyMTjofEmJypkms03CFiqrtHIGN1goE6BIMrCg25VieJKhLI0BmhE
4oviXJIwjdesjZq8eMWMo3bl2JtW+zyYTE7tRhC0mE0h8Hv1xDBVU1BVsqICrBi9aVI1UOoNzBi+
mioE3tl1HyCuZHOTvC3khBAdxDelgnq8Hf4udg87N/wb0Wqsfh63EKjEuPjbG2M6QQzyxY20QsqJ
CD6GnMuR/YJLHL6bleuLbg6ndLhs4abWyg3C4NwBX7TwMV+I1EvkgkuNDtLwyKQzWmvEI2bo+O5m
su6PVaYQpRstL3QlKpkjpXTerFAQWoBikwGeng2KGli9F40SyfVnAiHD6OMWTD/RyLQYWIUxqCRM
oTAIIOiQEWCt9FwjrIyEzNAjFO7QrQaRwuwmq4EBqRJSMCC6kX46rmlOyJtbjtGQYNGCYNXZkkl4
3NCJwm/gemumK+Danx1tmVhxFscK23S03IksQpgo+eVSd4VNg7yZw1/XLxsXkPUcTQor1zJtFqin
M2tW4GBWEDBoBlOlRUaDQ2aPeZmrQ/ZpVG0aiZz8C4ePBtiS0aKswuLsxHLXzPM6ywxpdhEntmcS
RYY6zsLPIRcQT0RJMsEReZ8w00eVbv9W2hZdTGZViiY/FUIwWxgZHRbUW6zGawIMSzIpCKwYpbgs
NkMjM0Iuzye/u7KYmWeZfC+HTJl2JtFvbbaGVRIR6A3tQdYctzmmsjfUcMjznTYWEsi4s0P/CKax
Fem/oOgkYhzEPUZl5m0lYzkyk5lvI84uLqq/lyRSW5/cyp+BveWw6zn8zpOCw5RG8pKseCGggGY2
UUw2QOvAZ6SYFVJTfscP6i/kiplYaziewoHaa9WWW7Wicdh4SyyLS223iOrKZ2U13YHBjZfquaNH
CG21JwNLZ85o5VmuJLjrN6ZrKHM0oUXA6Sps1wrcjCRlo7AMoROEOxzC0VlWhy4u5s9477h0RzDT
313jOmbmqnDCjcLT0WvCyjQGp40JZEXqR/gJCEYSiYM5kJU3qlGHOC9MhYs18/NziqFUap7P1EIY
NIgCIZ/We9BkfyGpFegvMhcgiCDnP8TjuQbwSg9xsFAHvJZfwFiuRCUCDBikVmYoxDUl72INw/wR
RZeDEQoCcTF+S1EMKhbBSHouCYPQfYDMMCRey4aXAZrDPADWOoNIUySwBozFqLyw0baGmMnS1Ysr
ciqzaxXM1mskQsAlCZeAbdysQXO804PCL7VDoRex6gDBH6A55aE4FDBEgZBobocyUCXu5k3Qcd3r
GgeR4j5F42l6A0saheUz9W1AZGp6XnYyKB8AHioeUfGCXMsAevVdhsLIJTCnqoZWyVdKWd7CrGIe
zTRSikWuJYxjgmExvAypTkYS4zoMAM1V06enC8gKsAR+w9WQ85zZ7q9niZjCM4YmgUJVJWMJmA4A
yFvi9dxUURTDBqS9GF+RFsYc067Cgn7TjGYAX7QvW/aZ27FY+BO+ynsTHZGC5MXB5NHeiiW4IvYB
DEtDQSgOqFjXQGe4kqQPgIjE4w79Bkw4ZBRwNtkOW25hphLIggZh9qplzpwC65G+xEbpgK3AVqDy
O84/bp3WcGmNnxaGYbzbZ7DbcEeKtI4yQPFliBXdztpOBY4HO5qNXTWh61jSBH1zK0REWyVDMjvp
dznZYxUZRgi1DCamwcE7QzmHZCEghxAYGgm4rlZDoAO5BJyou1fTDsuM0rRw6zEXj6ljVQsG1uth
2RSpTA41ZwOiYd+BxXsQzEHWIrfnQ2t0v5x2TCgPf5ezyk9sYU8CGs4Z5W0fBx8xWDLHSNMlVNWk
/a6t4zAX8alJHBl9Bvj1RI5zg2WgsbsbMiSJY261ezaeUAbYWbtDtUC8YQeFdzkF5WQS2ZeAGKSU
b53+MYpdIuBvOZ5DPhcyR22A2po8z713rNHodag898H3pgeAz01fqjr429rQmLq+lqgjz5lO4kUR
VpYNpRN49hwKjmK0QaeXUBoUVvi02wY09R/M8eZTQD6sCSzmJ/TeLegO6AD0YDY2lytj3NNg1AlC
oxoY2ZpRfhoRlgRtp6EhniEUu0tfNBrMXFR8UtLU8UnICR8JeltiItjfRkJhLrQa72mHAUxgb6He
KBcL++dux3P4zhkbzBfV7e3G+5pdJgla02xms3YgbOoXvqsHcFBHodqT45YpgnHGl6sumyCMZrKB
fmbwZOq1czMADGbEPSg7eJ5HgKdF7h+1oLd1GYDI699xQMSoq7ph3pxq76S0YF1dBOTCW9sqAeJy
QXiJLeIM0EeEFw+jG1DaSeiV8kfFk2j3PQ8LgvDgTzDz8d85erRERLb0C4AGQGIRRZkiTN7bVCxf
yIXh5jZHhWED9Y9b7w5TPgpXK4IuFpcHy2l0agwC68uMWfKMWEoIecDk0oSgHIaJDEhzhXYJqijb
FUcyLdpAnaElVVFA7spY2i3AW5skrbC5QJXtQxNWFOfK/yR2EC8TlBDk1HqQGYchIP0jQdPaMnbt
DwMCWNUBi7PwfkPa0DraQfFANFSftrWZisRGS0FIaoz7hw9PkgGScPZcXplnBuMH2dk9Kqbydddl
rQMsCOpl8oRAqEw3rPUd5QDus+DFZFqnT5EkXy+gutNczARwoI0zXv811lhUJMS1uG2M9sQrVCAq
A0s/eMGXTFzs8czfssU1GwZJpSxUKMFiMRx4GPf4QGoLRKtxjq96oSWJwCysHiWJUGWcIiox6DWo
m9fByM7YsYXSEFikyZmpZpHSo24ixkTcBT3EiVNJe3qgOHAIDdbjlE6XDJrTV0YI4vpESODOKuGD
oVujBtE5BRai7Yjf1hX8i+Tbb8Ft3ck43DWpiya2K/g7h7Not0kqVCdcDtC/rnjRVymS7Zf8OLUF
oa7KyiLmRV0U791GmcaxYzMMBnzGHpPCoR5iec0F1RMbaA24UQSCxKwKzIPGZC2fQR1dgbC8T9DU
FrPGiD8rEZsQRQhfJ2DCbIHAwYzCmnE10lzJWlK1klrLbng0G4NL0sVHSSoTaujFojrHM23RJY2r
u8T3e1n7Eu7cIO4mWhvOZAZaNIivWRunMOoPS4w6iHB5QFxoTepS83ixEZDtwwYsJ8i0jIIrXBYw
UsI/F849YjTiGZLLw1nIyohOWD62QVEjZ3BRgNZvtHDhh1GtiB4jY45BGagsSZInNZtDS1xa3/F3
JFOFCQ56GlHg


--===============7188243295943591729==--