diff options
author | Michael Widenius <monty@askmonty.org> | 2011-06-06 16:39:06 +0300 |
---|---|---|
committer | Michael Widenius <monty@askmonty.org> | 2011-06-06 16:39:06 +0300 |
commit | 6ae42b75b894b5c8a338536861601f2d6d3db686 (patch) | |
tree | 15dd677d92bbae3e363e0fdd2e4e878c2d9232bf /mysys/thr_lock.c | |
parent | 00752ba43c9034fd77e03a65f2a6be9ea15032d2 (diff) | |
download | mariadb-git-6ae42b75b894b5c8a338536861601f2d6d3db686.tar.gz |
Fixed lock sorting and lock check issues with thr_lock that caused warnings when running test suite.
Safety check that could cause core dump when doing create table with virtual column.
mysql-test/mysql-test-run.pl:
Show also warnings from thr_lock (which starts with just Warning, not Warning:)
mysql-test/r/lock.result:
Added test that showed not relevant warning when using table locks.
mysql-test/t/lock.test:
Added test that showed not relevant warning when using table locks.
mysys/thr_lock.c:
Fixed sorting of locks.
(Old sort code didn't handle case where TL_WRITE_CONCURRENT_INSERT must be sorted before TL_WRITE)
Added more information to check_locks warning output.
Fixed wrong testing of multiple different write locks for same table.
sql/item_cmpfunc.cc:
Safety check that could cause core dump when doing create table with virtual column.
Diffstat (limited to 'mysys/thr_lock.c')
-rw-r--r-- | mysys/thr_lock.c | 112 |
1 files changed, 70 insertions, 42 deletions
diff --git a/mysys/thr_lock.c b/mysys/thr_lock.c index f7908ab57bc..32593ca3f4f 100644 --- a/mysys/thr_lock.c +++ b/mysys/thr_lock.c @@ -112,25 +112,37 @@ static inline pthread_cond_t *get_cond(void) /* + Sort locks in priority order + + LOCK_CMP() + A First lock + B Second lock + + Return: + 0 if A >= B + 1 if A < B + Priority for locks (decides in which order locks are locked) We want all write locks to be first, followed by read locks. Locks from MERGE tables has a little lower priority than other locks, to allow one to release merge tables without having to unlock and re-lock other locks. The lower the number, the higher the priority for the lock. - Read locks should have 4, write locks should have 0. - UNLOCK is 8, to force these last in thr_merge_locks. For MERGE tables we add 2 (THR_LOCK_MERGE_PRIV) to the lock priority. THR_LOCK_LATE_PRIV (1) is used when one locks other tables to be merged with existing locks. This way we prioritize the original locks over the new locks. */ -static uint lock_priority[(uint)TL_WRITE_ONLY+1] = -{ 8, 4, 4, 4, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0}; - -#define LOCK_CMP(A,B) ((uchar*) ((A)->lock) + lock_priority[(uint) (A)->type] + (A)->priority < (uchar*) ((B)->lock) + lock_priority[(uint) (B)->type] + (B)->priority) +inline int LOCK_CMP(THR_LOCK_DATA *A, THR_LOCK_DATA *B) +{ + if (A->lock != B->lock) + return A->lock < B->lock; + if (A->type != B->type) + return A->type > B->type; /* Prioritize read */ + return A->priority < B->priority; +} /* For the future (now the thread specific cond is alloced by my_pthread.c) @@ -215,6 +227,7 @@ static int check_lock(struct st_lock_list *list, const char* lock_type, static void check_locks(THR_LOCK *lock, const char *where, + enum thr_lock_type type, my_bool allow_no_locks) { uint old_found_errors=found_errors; @@ -279,6 +292,7 @@ static void check_locks(THR_LOCK *lock, const char *where, found_errors++; fprintf(stderr, "Warning at '%s': Write lock %d waiting while no exclusive read locks\n",where,(int) lock->write_wait.data->type); + DBUG_PRINT("warning", ("Warning at '%s': Write lock %d waiting while no exclusive read locks\n",where,(int) lock->write_wait.data->type)); } } } @@ -293,8 +307,10 @@ static void check_locks(THR_LOCK *lock, const char *where, if (data->type != TL_WRITE_CONCURRENT_INSERT) { fprintf(stderr, - "Warning at '%s': Found TL_WRITE_CONCURRENT_INSERT lock mixed with other write locks\n", - where); + "Warning at '%s': Found TL_WRITE_CONCURRENT_INSERT lock mixed with other write lock: %d\n", + where, data->type); + DBUG_PRINT("warning", ("Warning at '%s': Found TL_WRITE_CONCURRENT_INSERT lock mixed with other write lock: %d\n", + where, data->type)); break; } } @@ -309,26 +325,34 @@ static void check_locks(THR_LOCK *lock, const char *where, fprintf(stderr, "Warning at '%s': Found WRITE_ALLOW_WRITE lock waiting for WRITE_ALLOW_WRITE lock\n", where); + DBUG_PRINT("warning", ("Warning at '%s': Found WRITE_ALLOW_WRITE lock waiting for WRITE_ALLOW_WRITE lock\n", + where)); + } } if (lock->read.data) { - if (!thr_lock_owner_equal(lock->write.data->owner, - lock->read.data->owner) && + THR_LOCK_DATA *data; + for (data=lock->read.data ; data ; data=data->next) + { + if (!thr_lock_owner_equal(lock->write.data->owner, + data->owner) && ((lock->write.data->type > TL_WRITE_DELAYED && lock->write.data->type != TL_WRITE_ONLY) || ((lock->write.data->type == TL_WRITE_CONCURRENT_INSERT || lock->write.data->type == TL_WRITE_ALLOW_WRITE) && - lock->read_no_write_count))) - { - found_errors++; - fprintf(stderr, - "Warning at '%s': Found lock of type %d that is write and read locked\n", - where, lock->write.data->type); - DBUG_PRINT("warning",("At '%s': Found lock of type %d that is write and read locked\n", - where, lock->write.data->type)); - - } + data->type == TL_READ_NO_INSERT))) + { + found_errors++; + fprintf(stderr, + "Warning at '%s' for lock: %d: Found lock of type %d that is write and read locked. Read_no_write_count: %d\n", + where, (int) type, lock->write.data->type, + lock->read_no_write_count); + DBUG_PRINT("warning",("At '%s' for lock %d: Found lock of type %d that is write and read locked\n", + where, (int) type, + lock->write.data->type)); + } + } } if (lock->read_wait.data) { @@ -354,7 +378,7 @@ static void check_locks(THR_LOCK *lock, const char *where, } #else /* EXTRA_DEBUG */ -#define check_locks(A,B,C) +#define check_locks(A,B,C,D) #endif @@ -531,13 +555,14 @@ wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data, else wait->last=data->prev; data->type= TL_UNLOCK; /* No lock */ - check_locks(data->lock, "killed or timed out wait_for_lock", 1); + check_locks(data->lock, "killed or timed out wait_for_lock", data->type, + 1); wake_up_waiters(data->lock); } else { DBUG_PRINT("thr_lock", ("lock aborted")); - check_locks(data->lock, "aborted wait_for_lock", 0); + check_locks(data->lock, "aborted wait_for_lock", data->type, 0); } } else @@ -546,7 +571,7 @@ wait_for_lock(struct st_lock_list *wait, THR_LOCK_DATA *data, if (data->lock->get_status) (*data->lock->get_status)(data->status_param, data->type == TL_WRITE_CONCURRENT_INSERT); - check_locks(data->lock,"got wait_for_lock",0); + check_locks(data->lock,"got wait_for_lock", data->type, 0); } pthread_mutex_unlock(&data->lock->mutex); @@ -579,7 +604,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner, (long) data, data->owner->info->thread_id, (long) lock, (int) lock_type)); check_locks(lock,(uint) lock_type <= (uint) TL_READ_NO_INSERT ? - "enter read_lock" : "enter write_lock",0); + "enter read_lock" : "enter write_lock", lock_type, 0); if ((int) lock_type <= (int) TL_READ_NO_INSERT) { /* Request for READ lock */ @@ -608,7 +633,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner, lock->read.last= &data->next; if (lock_type == TL_READ_NO_INSERT) lock->read_no_write_count++; - check_locks(lock,"read lock with old write lock",0); + check_locks(lock,"read lock with old write lock", lock_type, 0); if (lock->get_status) (*lock->get_status)(data->status_param, 0); statistic_increment(locks_immediate,&THR_LOCK_lock); @@ -632,7 +657,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner, lock->read.last= &data->next; if (lock_type == TL_READ_NO_INSERT) lock->read_no_write_count++; - check_locks(lock,"read lock with no write locks",0); + check_locks(lock,"read lock with no write locks", lock_type, 0); if (lock->get_status) (*lock->get_status)(data->status_param, 0); statistic_increment(locks_immediate,&THR_LOCK_lock); @@ -717,7 +742,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner, (*lock->write.last)=data; /* Add to running fifo */ data->prev=lock->write.last; lock->write.last= &data->next; - check_locks(lock,"second write lock",0); + check_locks(lock,"second write lock", lock_type, 0); if (lock->get_status) (*lock->get_status)(data->status_param, lock_type == TL_WRITE_CONCURRENT_INSERT); @@ -755,7 +780,7 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner, lock->write.last= &data->next; if (lock->get_status) (*lock->get_status)(data->status_param, concurrent_insert); - check_locks(lock,"only write lock",0); + check_locks(lock,"only write lock", lock_type, 0); statistic_increment(locks_immediate,&THR_LOCK_lock); goto end; } @@ -791,7 +816,7 @@ static inline void free_all_read_locks(THR_LOCK *lock, { THR_LOCK_DATA *data=lock->read_wait.data; - check_locks(lock,"before freeing read locks",1); + check_locks(lock,"before freeing read locks", TL_UNLOCK, 1); /* move all locks from read_wait list to read list */ (*lock->read.last)=data; @@ -833,7 +858,7 @@ static inline void free_all_read_locks(THR_LOCK *lock, *lock->read_wait.last=0; if (!lock->read_wait.data) lock->write_lock_count=0; - check_locks(lock,"after giving read locks",0); + check_locks(lock,"after giving read locks", TL_UNLOCK, 0); } /* Unlock lock and free next thread on same lock */ @@ -846,7 +871,7 @@ void thr_unlock(THR_LOCK_DATA *data, uint unlock_flags) DBUG_PRINT("lock",("data: 0x%lx thread: 0x%lx lock: 0x%lx", (long) data, data->owner->info->thread_id, (long) lock)); pthread_mutex_lock(&lock->mutex); - check_locks(lock,"start of release lock",0); + check_locks(lock,"start of release lock", lock_type, 0); if (((*data->prev)=data->next)) /* remove from lock-list */ data->next->prev= data->prev; @@ -880,9 +905,9 @@ void thr_unlock(THR_LOCK_DATA *data, uint unlock_flags) if (lock_type == TL_READ_NO_INSERT) lock->read_no_write_count--; data->type=TL_UNLOCK; /* Mark unlocked */ - check_locks(lock,"after releasing lock",1); + check_locks(lock,"after releasing lock", lock_type, 1); wake_up_waiters(lock); - check_locks(lock,"end of thr_unlock",1); + check_locks(lock,"end of thr_unlock", lock_type, 1); pthread_mutex_unlock(&lock->mutex); DBUG_VOID_RETURN; } @@ -1007,7 +1032,7 @@ static void wake_up_waiters(THR_LOCK *lock) free_all_read_locks(lock,0); } end: - check_locks(lock, "after waking up waiters", 0); + check_locks(lock, "after waking up waiters", TL_UNLOCK, 0); DBUG_VOID_RETURN; } @@ -1317,7 +1342,7 @@ void thr_downgrade_write_lock(THR_LOCK_DATA *in_data, DBUG_ASSERT(old_lock_type == TL_WRITE_ONLY); DBUG_ASSERT(old_lock_type > new_lock_type); in_data->type= new_lock_type; - check_locks(lock,"after downgrading lock",0); + check_locks(lock,"after downgrading lock", old_lock_type, 0); #if TO_BE_REMOVED switch (old_lock_type) @@ -1417,7 +1442,8 @@ void thr_downgrade_write_lock(THR_LOCK_DATA *in_data, data->prev= lock->write.last; lock->write.last= &data->next; data->next= 0; - check_locks(lock, "Started write lock after downgrade",0); + check_locks(lock, "Started write lock after downgrade", new_lock_type, + 0); data->cond= 0; pthread_cond_signal(cond); } @@ -1470,13 +1496,15 @@ void thr_downgrade_write_lock(THR_LOCK_DATA *in_data, if (data->type == TL_READ_NO_INSERT) lock->read_no_write_count++; - check_locks(lock, "Started read lock after downgrade",0); + check_locks(lock, "Started read lock after downgrade", new_lock_type, + 0); data->cond= 0; pthread_cond_signal(cond); } } } - check_locks(lock,"after starting waiters after downgrading lock",0); + check_locks(lock,"after starting waiters after downgrading lock", + new_lock_type, 0); #endif pthread_mutex_unlock(&lock->mutex); DBUG_VOID_RETURN; @@ -1497,7 +1525,7 @@ my_bool thr_upgrade_write_delay_lock(THR_LOCK_DATA *data, pthread_mutex_unlock(&lock->mutex); DBUG_RETURN(data->type == TL_UNLOCK); /* Test if Aborted */ } - check_locks(lock,"before upgrading lock",0); + check_locks(lock,"before upgrading lock", data->type, 0); /* TODO: Upgrade to TL_WRITE_CONCURRENT_INSERT in some cases */ data->type= new_lock_type; /* Upgrade lock */ @@ -1525,11 +1553,11 @@ my_bool thr_upgrade_write_delay_lock(THR_LOCK_DATA *data, lock->write_wait.last= &data->next; data->prev= &lock->write_wait.data; lock->write_wait.data=data; - check_locks(lock,"upgrading lock",0); + check_locks(lock,"upgrading lock", new_lock_type, 0); } else { - check_locks(lock,"waiting for lock",0); + check_locks(lock,"waiting for lock", new_lock_type, 0); } res= wait_for_lock(&lock->write_wait,data,1); if (res == THR_LOCK_SUCCESS && lock->start_trans) |