Commit 0d76cb7e authored by unknown's avatar unknown
Browse files

Reverted patch for new usage of open_count as it caused more problems than it solved 

Cleaned up patch for checking locks for multi-table updates


myisam/mi_close.c:
  Reverted patch for new usage of open_counts
myisam/mi_locking.c:
  Reverted patch for new usage of open_counts
sql/ha_myisam.cc:
  Reverted patch for new usage of open_counts
sql/handler.cc:
  Removed compiler warning
sql/sql_acl.cc:
  Removed compiler warning
sql/sql_table.cc:
  No need to unlock after failed call to external_lock()
sql/sql_update.cc:
  Cleaned up (and made it more secure) patch for checking locks for multi-table updates
parent c0364263

myisam/mi_close.c

+7 −2
Original line number Diff line number Diff line
@@ -70,8 +70,13 @@ int mi_close(register MI_INFO *info) 
      error=my_errno;

    if (share->kfile >= 0)

    {

      /* We must always flush the state with the current open_count. */

      if (share->mode != O_RDONLY)

      /*

        If we are crashed, we can safely flush the current state as it will

        not change the crashed state.

        We can NOT write the state in other cases as other threads

        may be using the file at this point

      */

      if (share->mode != O_RDONLY && mi_is_crashed(info))

	mi_state_info_write(share->kfile, &share->state, 1);

      if (my_close(share->kfile,MYF(0)))

        error = my_errno;

myisam/mi_locking.c

+34 −83
Original line number Diff line number Diff line
@@ -21,14 +21,24 @@ 
  isamdatabase.

*/



/*

  state.open_count in the .MYI file is used the following way:

  - For the first change of the file in this process it's incremented with

    mi_mark_file_change(). (We have a write lock on the file in this case)

  - In mi_close() it's decremented by _mi_decrement_open_count() if it

    was incremented in the same process.



  This mean that if we are the only process using the file, the open_count

  tells us if the MYISAM file wasn't properly closed. (This is true if

  my_disable_locking is set).

*/





#include "myisamdef.h"

#ifdef	__WIN__

#include <errno.h>

#endif



static int mi_unlock_open_count(MI_INFO *info, my_bool write_info);





	/* lock table by F_UNLCK, F_RDLCK or F_WRLCK */



int mi_lock_database(MI_INFO *info, int lock_type)
@@ -38,17 +48,17 @@ int mi_lock_database(MI_INFO *info, int lock_type) 
  MYISAM_SHARE *share=info->s;

  uint flag;

  DBUG_ENTER("mi_lock_database");

  DBUG_PRINT("enter",("mi_lock_database: lock_type %d, old lock %d"

                      ", r_locks %u, w_locks %u", lock_type,

                      info->lock_type, share->r_locks, share->w_locks));

  DBUG_PRINT("enter",("mi_lock_database: gl._changed %d, open_count %u '%s'",

  DBUG_PRINT("enter",("lock_type: %d  old lock %d  r_locks: %u  w_locks: %u "

                      "global_changed:  %d  open_count: %u  name: '%s'",

                      lock_type, info->lock_type, share->r_locks,

                      share->w_locks,

                      share->global_changed, share->state.open_count,

                      share->index_file_name));



  if (share->options & HA_OPTION_READ_ONLY_DATA ||

      info->lock_type == lock_type)

    DBUG_RETURN(0);

  if (lock_type == F_EXTRA_LCK)

  if (lock_type == F_EXTRA_LCK)                 /* Used by TMP tables */

  {

    ++share->w_locks;

    ++share->tot_locks;
@@ -90,8 +100,7 @@ int mi_lock_database(MI_INFO *info, int lock_type) 
	  share->state.process= share->last_process=share->this_process;

	  share->state.unique=   info->last_unique=  info->this_unique;

	  share->state.update_count= info->last_loop= ++info->this_loop;

          if (mi_unlock_open_count(info, FALSE) ||

              mi_state_info_write(share->kfile, &share->state, 1))

          if (mi_state_info_write(share->kfile, &share->state, 1))

	    error=my_errno;

	  share->changed=0;

	  if (myisam_flush)
@@ -106,19 +115,6 @@ int mi_lock_database(MI_INFO *info, int lock_type) 
	  if (error)

	    mi_mark_crashed(info);

	}

        else

        {

          /*

            There are chances that _mi_mark_file_changed() has been called,

            while share->changed remained FALSE. Consequently, we need to

            clear the open_count even when share->changed is FALSE. Note,

            that mi_unlock_open_count() will only clear the open_count when

            it is set and only write the status to file, if it changes it

            and we are running --with-external-locking.

          */

          if (mi_unlock_open_count(info, ! my_disable_locking))

            error= my_errno;

        }

	if (info->lock_type != F_EXTRA_LCK)

	{

	  if (share->r_locks)
@@ -142,16 +138,17 @@ int mi_lock_database(MI_INFO *info, int lock_type) 
      break;

    case F_RDLCK:

      if (info->lock_type == F_WRLCK)

      {						/* Change RW to READONLY */

      {

        /*

          Change RW to READONLY



          mysqld does not turn write locks to read locks,

          so we're never here in mysqld.

        */

	if (share->w_locks == 1)

	{

	  flag=1;

          if (mi_unlock_open_count(info, ! my_disable_locking) ||

              my_lock(share->kfile,lock_type,0L,F_TO_EOF,

          if (my_lock(share->kfile,lock_type,0L,F_TO_EOF,

		      MYF(MY_SEEK_NOT_DONE)))

	  {

	    error=my_errno;
@@ -179,14 +176,6 @@ int mi_lock_database(MI_INFO *info, int lock_type) 
	  my_errno=error;

	  break;

	}

        if (share->state.open_count)

        {

          DBUG_PRINT("error",("RD: Table has not been correctly unlocked"

                              ": open_count %d '%s'",

                              share->state.open_count,

                              share->index_file_name));

          mi_mark_crashed(info);

        }

      }

      VOID(_mi_test_if_changed(info));

      share->r_locks++;
@@ -232,14 +221,6 @@ int mi_lock_database(MI_INFO *info, int lock_type) 
	      my_errno=error;

	      break;

	    }

            if (share->state.open_count)

            {

              DBUG_PRINT("error",("WR: Table has not been correctly unlocked"

                                  ": open_count %d '%s'",

                                  share->state.open_count,

                                  share->index_file_name));

              mi_mark_crashed(info);

            }

	  }

	}

      }
@@ -375,9 +356,10 @@ int _mi_readinfo(register MI_INFO *info, int lock_type, int check_keybuffer) 
} /* _mi_readinfo */





	/* Every isam-function that uppdates the isam-database must! end */

	/* with this request */

	/* ARGSUSED */

/*

  Every isam-function that uppdates the isam-database MUST end with this

  request

*/



int _mi_writeinfo(register MI_INFO *info, uint operation)

{
@@ -450,6 +432,8 @@ int _mi_mark_file_changed(MI_INFO *info) 
{

  char buff[3];

  register MYISAM_SHARE *share=info->s;

  DBUG_ENTER("_mi_mark_file_changed");



  if (!(share->state.changed & STATE_CHANGED) || ! share->global_changed)

  {

    share->state.changed|=(STATE_CHANGED | STATE_NOT_ANALYZED |
@@ -463,12 +447,12 @@ int _mi_mark_file_changed(MI_INFO *info) 
    {

      mi_int2store(buff,share->state.open_count);

      buff[2]=1;				/* Mark that it's changed */

      return (my_pwrite(share->kfile,buff,sizeof(buff),

      DBUG_RETURN(my_pwrite(share->kfile,buff,sizeof(buff),

                            sizeof(share->state.header),

                            MYF(MY_NABP)));

    }

  }

  return 0;

  DBUG_RETURN(0);

}
@@ -501,36 +485,3 @@ int _mi_decrement_open_count(MI_INFO *info) 
  }

  return test(lock_error || write_error);

}
 


/*

  Decrement open_count in preparation for unlock.



  SYNOPSIS

    mi_unlock_open_count()

    info                        Pointer to the MI_INFO structure.

    write_info                  If info must be written when changed.



  RETURN

    0     OK

*/



static int mi_unlock_open_count(MI_INFO *info, my_bool write_info)

{

  int rc= 0;

  MYISAM_SHARE *share=info->s;



  DBUG_ENTER("mi_unlock_open_count");

  DBUG_PRINT("enter",("mi_unlock_open_count: gl._changed %d open_count %d '%s'",

                      share->global_changed, share->state.open_count,

                      share->index_file_name));

  if (share->global_changed)

  {

    share->global_changed= 0;

    if (share->state.open_count)

      share->state.open_count--;

    if (write_info)

      rc= _mi_writeinfo(info, WRITEINFO_UPDATE_KEYFILE);

  }

  DBUG_RETURN(rc);

}

sql/ha_myisam.cc

+4 −22
Original line number Diff line number Diff line
@@ -998,31 +998,13 @@ int ha_myisam::delete_table(const char *name) 
  return mi_delete_table(name);

}





int ha_myisam::external_lock(THD *thd, int lock_type)

{

  int rc;



  while ((! (rc= mi_lock_database(file, !table->tmp_table ?

  return mi_lock_database(file, !table->tmp_table ?

			  lock_type : ((lock_type == F_UNLCK) ?

                                               F_UNLCK : F_EXTRA_LCK)))) &&

         mi_is_crashed(file) && (myisam_recover_options != HA_RECOVER_NONE))

  {

    /*

      check_and_repair() implicitly write locks the table, unless a

      LOCK TABLES is in effect. It should be safer to always write lock here.

      The implicit lock by check_and_repair() will then be a no-op.

      check_and_repair() does not restore the original lock, but unlocks the

      table. So we have to get the requested lock type again. And then to 

      check, if the table has been crashed again meanwhile by another server.

      If anything fails, we break.

    */

    if (((lock_type != F_WRLCK) && (rc= mi_lock_database(file, F_WRLCK))) ||

        (rc= check_and_repair(thd)))

      break;

				       F_UNLCK : F_EXTRA_LCK));

}

  return rc;

}





THR_LOCK_DATA **ha_myisam::store_lock(THD *thd,

				      THR_LOCK_DATA **to,

sql/handler.cc

+1 −1
Original line number Diff line number Diff line
@@ -348,7 +348,7 @@ int ha_commit_trans(THD *thd, THD_TRANS* trans) 
    if (trans == &thd->transaction.all &&

	my_b_tell(&thd->transaction.trans_log))

    {

      if (error= wait_if_global_read_lock(thd, 0, 0))

      if ((error= wait_if_global_read_lock(thd, 0, 0)))

      {

        /*

          Note that ROLLBACK [TO SAVEPOINT] does not have this test; it's

sql/sql_acl.cc

+1 −1
Original line number Diff line number Diff line
@@ -469,7 +469,7 @@ static ulong get_sort(uint count,...) 
    uint chars= 0;

    uint wild_pos= 0;           /* first wildcard position */



    if (start= str)

    if ((start= str))

    {

      for (; *str ; str++)

      {