List:Summer of Code« Previous MessageNext Message »
From:Sergei Golubchik Date:August 6 2009 10:42am
Subject:Re: GSoC Week 13 - I_S/P_S storage engine
View as plain text  
Hi, Haihao!

On Aug 04, Haihao Tang wrote:
> 
> I have fixed bugs of TABLE and VIEWS. But new bug is shown.
> If run "SELECT * FROM information_schema.TABLES" or "SELECT * FROM
> INFOSCHEMA.TABLES" more than 2 times,
> MySQL server will crash on "Field::clone". The problem should be issued by
> infoschema_discover, which does not set field->ptr right, like
> the bug we meet before. But I check infoschema_discover many times, I can't
> find what is wrong.

Step-by-step:
1. start mysqld in gdb
2. run your queries, let it crash
3. it crashes on

       if ((tmp= (Field*) memdup_root(root,(char*) this,size_of())))

4. why ? there's only one possibility. Note that size_of() is really
   this->size_of(). So, 'this' content must be wrong.
5. (gdb) p this[0]
6. note that it's filled with zeros, vtbl is zero, all pointers are
   nulls, and so on.
7. restart mysqld, breakpoint on infoschema_discover()
8. run your query, wait for the discovery of "TABLES"
9. go down, until all fields are created (gdb 'u' command helps here)
10. check any field:

   (gdb) p share->field[0][0]

11. it's ok. wait till it's changed (using, for example) charset field:
    (gdb) p &share->field[0]->field_charset
    $10 = (CHARSET_INFO **)0x12345678
    (gdb) watch *(CHARSET_INFO **)0x12345678
    (gdb) c
12. you'll see that the value will be changed here:

      free_root(thd->mem_root,MYF(MY_KEEP_PREALLOC));

    in dispatch_command(), sql_parse.cc

now it's all clear. See the new() method of the Field class - it uses
sql_alloc(), which allocates from the current thread's mem_root. Usually
current thread's mem_root is thd->mem_root. It's being emptied at the
end of every query - and all your fields evaporate at that moment.

You need your fields allocated on share->mem_root. See now it's done in
open_table_def():

    root_ptr= my_pthread_getspecific_ptr(MEM_ROOT**, THR_MALLOC);
    old_root= *root_ptr;
    *root_ptr= &share->mem_root;
    error= open_binary_frm(thd, share, head, file);
    *root_ptr= old_root;

you need to do the same, temporaliry changing current memroot to be your
share->mem_root. This is the patch:

--- storage/infoschema/ha_infoschema.cc 2009-07-27 12:24:20 +0000
+++ storage/infoschema/ha_infoschema.cc 2009-08-06 09:58:22 +0000
@@ -225,6 +225,10 @@ TABLE_SHARE * infoschema_discover(handle
   int reclength= 0, null_count= 0, blob_count= 0;
   int fieldnr= 0;
 
+  MEM_ROOT **root_ptr= my_pthread_getspecific_ptr(MEM_ROOT**, THR_MALLOC);
+  MEM_ROOT *old_root= *root_ptr;
+  *root_ptr= &share->mem_root;
+
   for(; fields_info->field_name; fields_info++) 
   {
     Field *new_field= NULL;
@@ -371,6 +375,8 @@ TABLE_SHARE * infoschema_discover(handle
     *(reg_field++)= new_field;
   }
 
+  *root_ptr= old_root;
+
   DBUG_ASSERT(fieldnr == (uint)(reg_field - share->field));
   DBUG_ASSERT(fieldnr >= field_count);
   *reg_field= 0;

One problem with this fix - you have one return from inside the for()
loop, and it'll return without restoring old *root_ptr. Don't forget to
fix it when applying this patch above.

Two more issues. I constantly see errors like

Error: Freeing pointer out of range at line 312, 'ha_archive.cc'
Error: Freeing pointer out of range at line 312, 'ha_archive.cc'
Error: Freeing pointer out of range at line 312, 'ha_archive.cc'
Error: Freeing pointer out of range at line 312, 'ha_archive.cc'

please make sure you build with safemalloc to catch problems
like that. In this case it was uninitialized pointer passed to
my_free(). This is the fix:

=== modified file 'storage/archive/ha_archive.cc'
--- storage/archive/ha_archive.cc       2009-07-16 01:11:13 +0000
+++ storage/archive/ha_archive.cc       2009-08-06 10:03:10 +0000
@@ -273,7 +273,7 @@ TABLE_SHARE * archive_discover(handlerto
   DBUG_PRINT("archive_discover", ("db: %s, name: %s", db, name)); 
   azio_stream frm_stream;
   char az_file[FN_REFLEN];
-  char *frm_ptr;
+  char *frm_ptr= 0;
   MY_STAT file_stat;
   char path[FN_REFLEN];
   TABLE_SHARE *found_share = NULL;

And another problem. Test case
mysql> SELECT * FROM information_schema.TABLES where table_name='VIEWS';
+---------------+--------------------+------------+-------------+------------+...
| TABLE_CATALOG | TABLE_SCHEMA       | TABLE_NAME | TABLE_TYPE  | ENGINE     |...
+---------------+--------------------+------------+-------------+------------+...
| def           | information_schema | VIEWS      | SYSTEM VIEW | MARIA      |...
| def           | INFOSCHEMA         | VIEWS      | BASE TABLE  | INFOSCHEMA |...
| def           | INFOSCHEMA         | VIEWS      | BASE TABLE  | INFOSCHEMA |...
+---------------+--------------------+------------+-------------+------------+...

your VIEWS table is shown twice. If I_S cannot optimize the expression,
like in

 SELECT * FROM information_schema.TABLES;

or

 SELECT * FROM information_schema.TABLES where concat(table_name)='VIEWS';

it appears only once, as expected.

Regards / Mit vielen Grüßen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Engineer/Server Architect
/_/  /_/\_, /___/\___\_\___/  Sun Microsystems GmbH, HRB München 161028
       <___/                  Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin Häring
Thread
Re: GSoC Week 13 - I_S/P_S storage engineHaihao Tang4 Aug
  • Re: GSoC Week 13 - I_S/P_S storage engineSergei Golubchik6 Aug
  • Re: GSoC Week 13 - I_S/P_S storage engineSergei Golubchik6 Aug