On 07/10/10 10:55, Konstantin Osipov wrote:
> * Magne Mahre <magne.mahre@stripped> [10/09/30 14:26]:
>> === modified file 'storage/myisam/mi_panic.c'
>> --- a/storage/myisam/mi_panic.c 2009-12-05 01:26:15 +0000
>> +++ b/storage/myisam/mi_panic.c 2010-09-30 09:29:51 +0000
>
> I don't understand how it works at all.
>
> mi_close() will erase the table entry from the hash. This
> invalidates the iterator. I think for () loop needs to be changed
Agreed. I've changed the iteration.
> Besides, in a fully concurrent environment, what is the guarantee
> that mi_close() is not called twice on the same info? (The old
> code seems to also have been prone to this bug).
>
> I mean specifically when mi_panic is called simultaneously from
> two threads:
>
> thread1: take THR_LOCK_myisam, get the first record, release THR_LOCK_myisam
> thread2: take THR_LOCK_myisam, get the first record, release THR_LOCK_myisam
> thread1: mi_close first record
> thread2: mi_close first record.
>
> I think we need something like "panic_in_progress" to prevent
> this situation, or add a comment that mi_panic should never be
> called concurrently with itself.
I did not address this issue. As svoj notes, this is an
extremely rare situation. If you insist, I'll do it :)
--Magne
