Commit 7553f9ee authored by Marc Alff's avatar Marc Alff

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
parent fe12e0b2
...@@ -211,13 +211,16 @@ void _lf_pinbox_put_pins(LF_PINS *pins) ...@@ -211,13 +211,16 @@ void _lf_pinbox_put_pins(LF_PINS *pins)
LF_PINBOX *pinbox= pins->pinbox; LF_PINBOX *pinbox= pins->pinbox;
uint32 top_ver, nr; uint32 top_ver, nr;
nr= pins->link; nr= pins->link;
#ifdef MY_LF_EXTRA_DEBUG
#ifndef DBUG_OFF
{ {
/* This thread should not hold any pin. */
int i; int i;
for (i= 0; i < LF_PINBOX_PINS; i++) for (i= 0; i < LF_PINBOX_PINS; i++)
DBUG_ASSERT(pins->pin[i] == 0); DBUG_ASSERT(pins->pin[i] == 0);
} }
#endif #endif /* DBUG_OFF */
/* /*
XXX this will deadlock if other threads will wait for XXX this will deadlock if other threads will wait for
the caller to do something after _lf_pinbox_put_pins(), the caller to do something after _lf_pinbox_put_pins(),
......
/* 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 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 it under the terms of the GNU General Public License as published by
...@@ -801,6 +801,22 @@ void destroy_thread(PFS_thread *pfs) ...@@ -801,6 +801,22 @@ void destroy_thread(PFS_thread *pfs)
pfs->m_lock.allocated_to_free(); pfs->m_lock.allocated_to_free();
} }
/**
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. Find or create instrumentation for a file instance by file name.
@param thread the executing instrumented thread @param thread the executing instrumented thread
...@@ -816,22 +832,12 @@ find_or_create_file(PFS_thread *thread, PFS_file_class *klass, ...@@ -816,22 +832,12 @@ find_or_create_file(PFS_thread *thread, PFS_file_class *klass,
PFS_file *pfs; PFS_file *pfs;
PFS_scan scan; 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++; file_lost++;
return NULL; return NULL;
} }
}
char safe_buffer[FN_REFLEN]; char safe_buffer[FN_REFLEN];
const char *safe_filename; const char *safe_filename;
...@@ -904,7 +910,7 @@ find_or_create_file(PFS_thread *thread, PFS_file_class *klass, ...@@ -904,7 +910,7 @@ find_or_create_file(PFS_thread *thread, PFS_file_class *klass,
/* Append the unresolved file name to the resolved path */ /* Append the unresolved file name to the resolved path */
char *ptr= buffer + strlen(buffer); char *ptr= buffer + strlen(buffer);
char *buf_end= &buffer[sizeof(buffer)-1]; char *buf_end= &buffer[sizeof(buffer)-1];
if (buf_end > ptr) if ((buf_end > ptr) && (*(ptr-1) != FN_LIBCHAR))
*ptr++= FN_LIBCHAR; *ptr++= FN_LIBCHAR;
if (buf_end > ptr) if (buf_end > ptr)
strncpy(ptr, safe_filename + dirlen, buf_end - ptr); strncpy(ptr, safe_filename + dirlen, buf_end - ptr);
...@@ -918,16 +924,18 @@ find_or_create_file(PFS_thread *thread, PFS_file_class *klass, ...@@ -918,16 +924,18 @@ find_or_create_file(PFS_thread *thread, PFS_file_class *klass,
const uint retry_max= 3; const uint retry_max= 3;
search: search:
entry= reinterpret_cast<PFS_file**> entry= reinterpret_cast<PFS_file**>
(lf_hash_search(&filename_hash, thread->m_filename_hash_pins, (lf_hash_search(&filename_hash, pins,
normalized_filename, normalized_length)); normalized_filename, normalized_length));
if (entry && (entry != MY_ERRPTR)) if (entry && (entry != MY_ERRPTR))
{ {
pfs= *entry; pfs= *entry;
pfs->m_file_stat.m_open_count++; pfs->m_file_stat.m_open_count++;
lf_hash_search_unpin(thread->m_filename_hash_pins); lf_hash_search_unpin(pins);
return pfs; return pfs;
} }
lf_hash_search_unpin(pins);
/* filename is not constant, just using it for noise on create */ /* filename is not constant, just using it for noise on create */
uint random= randomized_index(filename, file_max); uint random= randomized_index(filename, file_max);
...@@ -954,7 +962,7 @@ search: ...@@ -954,7 +962,7 @@ search:
reset_single_stat_link(&pfs->m_wait_stat); reset_single_stat_link(&pfs->m_wait_stat);
int res; int res;
res= lf_hash_insert(&filename_hash, thread->m_filename_hash_pins, res= lf_hash_insert(&filename_hash, pins,
&pfs); &pfs);
if (likely(res == 0)) if (likely(res == 0))
{ {
...@@ -1006,9 +1014,12 @@ void release_file(PFS_file *pfs) ...@@ -1006,9 +1014,12 @@ void release_file(PFS_file *pfs)
void destroy_file(PFS_thread *thread, PFS_file *pfs) void destroy_file(PFS_thread *thread, PFS_file *pfs)
{ {
DBUG_ASSERT(thread != NULL); DBUG_ASSERT(thread != NULL);
DBUG_ASSERT(thread->m_filename_hash_pins != NULL);
DBUG_ASSERT(pfs != 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_filename, pfs->m_filename_length);
pfs->m_lock.allocated_to_free(); pfs->m_lock.allocated_to_free();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment