BUG#23527: set global query_cache_size can crash the server under

           high load

MySQL server could crash if two or more threads would initiate query
cache resize at the moments very close in time.

The problem was introduced with the fix of bug 21051 in 5.0 and 5.1:
simultaneous query cache resizes would wait for the first one in
progress, but then each thread would try to finish the operation,
accessing the data that was already reset (attempt to dereference
'bins' pointer, which may be NULL already).

The solution is to check after synchronization if another thread has
done the reset already (test 'query_cache_size > 0' again).

No test case is provided because the bug is a subject to a race.


sql/sql_cache.cc:
  We release 'structure_guard_mutex' in flush_cache(), so after the
  call we check if another thread had reset the cache before us.

1 files changed, 17 insertions, 1 deletions

diff --git a/sql/sql_cache.cc b/sql/sql_cache.cc
index 635c65eb726..47632f6f105 100644
--- a/sql/sql_cache.cc
+++ b/sql/sql_cache.cc
@@ -1766,8 +1766,18 @@ void Query_cache::free_cache()
 {
   DBUG_ENTER("Query_cache::free_cache");
   if (query_cache_size > 0)
-  {
     flush_cache();
+  /*
+    There may be two free_cache() calls in progress, because we
+    release 'structure_guard_mutex' in flush_cache().  When the second
+    flush_cache() wakes up from the wait on 'COND_flush_finished', the
+    first call to free_cache() has done its job.  So we have to test
+    'query_cache_size > 0' the second time to see if the cache wasn't
+    reset by other thread, or if it was reset and was re-enabled then.
+    If the cache was reset, then we have nothing to do here.
+  */
+  if (query_cache_size > 0)
+  {
 #ifndef DBUG_OFF
     if (bins[0].free_blocks == 0)
     {
@@ -1809,6 +1819,12 @@ void Query_cache::free_cache()
     flush_in_progress flag and releases the lock, so other threads may
     proceed skipping the cache as if it is disabled.  Concurrent
     flushes are performed in turn.
+
+    After flush_cache() call, the cache is flushed, all the freed
+    memory is accumulated in bin[0], and the 'structure_guard_mutex'
+    is locked.  However, since we could release the mutex during
+    execution, the rest of the cache state could have been changed,
+    and should not be relied on.
 */
 
 void Query_cache::flush_cache()