[asterisk-dev] [svn-commits] mmichelson: trunk r125133 - /trunk/include/asterisk/lock.h

[Russell Bryant]

On Jun 25, 2008, at 6:25 PM, SVN commits to the Digium repositories  
wrote:

> Author: mmichelson
> Date: Wed Jun 25 17:25:20 2008
> New Revision: 125133
>
> URL: http://svn.digium.com/view/asterisk?view=rev&rev=125133
> Log:
> Fix a bug in the rwlock tracking. ast_rwlock_unlock did not take into
> account that multiple threads could hold the same rdlock at the same  
> time.
> As such, it expected that when a thread released a lock that it must  
> have
> been the last to acquire the lock as well. Erroneous error messages  
> would
> be sent to the console stating that a thread was attempting to  
> unlock a lock
> it did not own.
>
> Now all threads are examined to be sure that the message is only  
> printed
> when it is supposed to be printed.
>
>
> Modified:
>    trunk/include/asterisk/lock.h
>
> Modified: trunk/include/asterisk/lock.h
> URL: http://svn.digium.com/view/asterisk/trunk/include/asterisk/lock.h?view=diff&rev=125133&r1=125132&r2=125133
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- trunk/include/asterisk/lock.h (original)
> +++ trunk/include/asterisk/lock.h Wed Jun 25 17:25:20 2008
> @@ -1048,28 +1048,37 @@
> #endif /* AST_MUTEX_INIT_W_CONSTRUCTORS */
> 	
> 	ast_reentrancy_lock(lt);
> -	if (lt->reentrancy && (lt->thread[lt->reentrancy-1] !=  
> pthread_self())) {
> -		__ast_mutex_logger("%s line %d (%s): attempted unlock rwlock '%s'  
> without owning it!\n",
> -					filename, line, func, name);
> -		__ast_mutex_logger("%s line %d (%s): '%s' was locked here.\n",
> -				lt->file[lt->reentrancy-1], lt->lineno[lt->reentrancy-1], lt- 
> >func[lt->reentrancy-1], name);
> +	if (lt->reentrancy) {
> +		int lock_found = 0;
> +		int i;
> +		for (i = lt->reentrancy-1; i >= 0; --i) {
> +			if (lt->thread[i] == pthread_self()) {
> +				lock_found = 1;
> +				if (i != lt->reentrancy-1) {
> +					lt->file[i] = lt->file[lt->reentrancy-1];
> +					lt->lineno[i] = lt->lineno[lt->reentrancy-1];
> +					lt->func[i] = lt->func[lt->reentrancy-1];
> +					lt->thread[i] = lt->thread[lt->reentrancy-1];
> +				}
> +				break;
> +			}
> +		}
> +		if (!lock_found) {
> +			__ast_mutex_logger("%s line %d (%s): attempted unlock rwlock  
> '%s' without owning it!\n",
> +						filename, line, func, name);
> +			__ast_mutex_logger("%s line %d (%s): '%s' was last locked here. 
> \n",
> +					lt->file[lt->reentrancy-1], lt->lineno[lt->reentrancy-1], lt- 
> >func[lt->reentrancy-1], name);


This is a minor tweak, but you don't need the "lock_found" variable.   
Instead of if (!lock_found), you can check if (i < 0).

One other thing, it would be good to store the result of  
pthread_self(), instead of calling it on every iteration of the loop.

--
Russell Bryant
Senior Software Engineer
Open Source Team Lead
Digium, Inc.