From: Marc Alff Date: March 28 2012 4:15pm Subject: bzr push into mysql-5.5 branch (marc.alff:3772 to 3773) Bug#13898343 List-Archive: http://lists.mysql.com/commits/143350 X-Bug: 13898343 Message-Id: <201203281615.q2SGFNx2019335@acsmt357.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3773 Marc Alff 2012-03-28 Bug#13898343 THREAD LOOPS ENDLESSLY IN LF_PINBOX_PUT_PINS WHILE HOLDING LOCK_THREAD_COUNT When using the performance schema file io instrumentation in MySQL 5.5, a thread would loop forever inside lf_pinbox_put_pins, when disconnecting. It would also hold LOCK_thread_count while doing so, effectively killing the server. The root cause of the loop in lf_pinbox_put_pins() is a leak of LF_PINS, when used with the filename_hash LF_HASH table in the performance schema. This fix contains the following changes: 1) Added the missing call to lf_hash_search_unpin(), to prevent the leak. 2) In mysys/lf_alloc-pin.c, there was some extra debugging code (MY_LF_EXTRA_DEBUG) written to detect precisely this kind of issues, but it was never used. Replaced MY_LF_EXTRA_DEBUG with DBUG_OFF, so that leaks similar to this one can be always detected in regular debug builds. 3) Backported the fix for the following bug, from 5.6 to 5.5: Bug#13417446 - 63339: INCORRECT FILE PATH IN PEFORMANCE_SCHEMA ON WINDOWS modified: mysys/lf_alloc-pin.c storage/perfschema/pfs_instr.cc 3772 Praveenkumar Hulakund 2012-03-28 [merge] Merge from 5.1 to 5.5 modified: mysql-test/r/sp-bugs.result mysql-test/r/sp-code.result mysql-test/r/sp_notembedded.result mysql-test/t/sp-bugs.test mysql-test/t/sp-code.test mysql-test/t/sp_notembedded.test === modified file 'mysys/lf_alloc-pin.c' --- a/mysys/lf_alloc-pin.c 2011-06-30 15:46:53 +0000 +++ b/mysys/lf_alloc-pin.c 2012-03-28 15:54:30 +0000 @@ -211,13 +211,16 @@ void _lf_pinbox_put_pins(LF_PINS *pins) LF_PINBOX *pinbox= pins->pinbox; uint32 top_ver, nr; nr= pins->link; -#ifdef MY_LF_EXTRA_DEBUG + +#ifndef DBUG_OFF { + /* This thread should not hold any pin. */ int i; for (i= 0; i < LF_PINBOX_PINS; i++) DBUG_ASSERT(pins->pin[i] == 0); } -#endif +#endif /* DBUG_OFF */ + /* XXX this will deadlock if other threads will wait for the caller to do something after _lf_pinbox_put_pins(), === modified file 'storage/perfschema/pfs_instr.cc' --- a/storage/perfschema/pfs_instr.cc 2011-06-30 15:46:53 +0000 +++ b/storage/perfschema/pfs_instr.cc 2012-03-28 15:54:30 +0000 @@ -1,4 +1,4 @@ -/* Copyright (c) 2008, 2010, Oracle and/or its affiliates. All rights reserved. +/* Copyright (c) 2008, 2012, Oracle and/or its affiliates. All rights reserved. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -802,6 +802,22 @@ void destroy_thread(PFS_thread *pfs) } /** + Get the hash pins for @filename_hash. + @param thread The running thread. + @returns The LF_HASH pins for the thread. +*/ +LF_PINS* get_filename_hash_pins(PFS_thread *thread) +{ + if (unlikely(thread->m_filename_hash_pins == NULL)) + { + if (! filename_hash_inited) + return NULL; + thread->m_filename_hash_pins= lf_hash_get_pins(&filename_hash); + } + return thread->m_filename_hash_pins; +} + +/** Find or create instrumentation for a file instance by file name. @param thread the executing instrumented thread @param klass the file class @@ -816,23 +832,13 @@ find_or_create_file(PFS_thread *thread, PFS_file *pfs; PFS_scan scan; - if (! filename_hash_inited) + LF_PINS *pins= get_filename_hash_pins(thread); + if (unlikely(pins == NULL)) { - /* File instrumentation can be turned off. */ file_lost++; return NULL; } - if (unlikely(thread->m_filename_hash_pins == NULL)) - { - thread->m_filename_hash_pins= lf_hash_get_pins(&filename_hash); - if (unlikely(thread->m_filename_hash_pins == NULL)) - { - file_lost++; - return NULL; - } - } - char safe_buffer[FN_REFLEN]; const char *safe_filename; @@ -904,7 +910,7 @@ find_or_create_file(PFS_thread *thread, /* Append the unresolved file name to the resolved path */ char *ptr= buffer + strlen(buffer); char *buf_end= &buffer[sizeof(buffer)-1]; - if (buf_end > ptr) + if ((buf_end > ptr) && (*(ptr-1) != FN_LIBCHAR)) *ptr++= FN_LIBCHAR; if (buf_end > ptr) strncpy(ptr, safe_filename + dirlen, buf_end - ptr); @@ -918,16 +924,18 @@ find_or_create_file(PFS_thread *thread, const uint retry_max= 3; search: entry= reinterpret_cast - (lf_hash_search(&filename_hash, thread->m_filename_hash_pins, + (lf_hash_search(&filename_hash, pins, normalized_filename, normalized_length)); if (entry && (entry != MY_ERRPTR)) { pfs= *entry; pfs->m_file_stat.m_open_count++; - lf_hash_search_unpin(thread->m_filename_hash_pins); + lf_hash_search_unpin(pins); return pfs; } + lf_hash_search_unpin(pins); + /* filename is not constant, just using it for noise on create */ uint random= randomized_index(filename, file_max); @@ -954,7 +962,7 @@ search: reset_single_stat_link(&pfs->m_wait_stat); int res; - res= lf_hash_insert(&filename_hash, thread->m_filename_hash_pins, + res= lf_hash_insert(&filename_hash, pins, &pfs); if (likely(res == 0)) { @@ -1006,9 +1014,12 @@ void release_file(PFS_file *pfs) void destroy_file(PFS_thread *thread, PFS_file *pfs) { DBUG_ASSERT(thread != NULL); - DBUG_ASSERT(thread->m_filename_hash_pins != NULL); DBUG_ASSERT(pfs != NULL); - lf_hash_delete(&filename_hash, thread->m_filename_hash_pins, + + LF_PINS *pins= get_filename_hash_pins(thread); + DBUG_ASSERT(pins != NULL); + + lf_hash_delete(&filename_hash, pins, pfs->m_filename, pfs->m_filename_length); pfs->m_lock.allocated_to_free(); } No bundle (reason: useless for push emails).