[Date Prev][Date Next] [Chronological] [Thread] [Top]

potential race condition in modify operation



It seems likely that there is multiple areas where we may have a race
condition when multiple connections to slapd attempt to modify the 
same entry.

Specifically there seems to not be (I can't find any) any mechanism for 
preventing two threads from starting a modify (for example) of an entry. 
This apparantly can lead to various problems such as (but not limited to):

	- whose change's get done 
	- if one processes changes fail due to schema check do the
	  other threads changes get forgotton properly
	- what happens when two threads start to do low level changes
	  to the attribute values and pointers thereto possibly clobbering
	  each other

We noticed this when ted saw an error when trying to add an attribute not
allowed in the schema. The operation failed. But subsequent operations 
showed that the value was in the entry. Until the server was restarted
after which the attribute disappeared. back-ldbm/modify.c was doing
a:
	 cache_return_entry( &li->li_cache, e ); 

which in turn is supposed to tell the cache the entry is no longer needed.
Unfortunately the cache code leaves it around until space is needed which
means that if you request the entry again it simply gives you the
(incorrectly) modified verion already in the cache.

To fix this I added:

 	if ( cache_delete_entry( &li->li_cache, e ) != 0 )
                     Debug( LDAP_DEBUG_ANY, "could not delete %d (%s) from cache\n", e->e_id, e->e_dn, 0 );

just before the cache_return_entry() call. This works. But while we where 
looking at this it became apparant that unless we're missing something
there is no protection from multiple threads accessing and modifying entries
at the same time. For example with this change, assuming two threads
are in the process of modifying an entry, do your changes get updated
by the other thread, do his changes get thrown out with yours, does
he get SIGSEGV when he attempts to use the (possibly now deleted) entry.

I would think that we need a per entry mutex. This could be used to lock
threads from using the entry when it is being modified. 


-- 
Stuart Lynne <sl@fireplug.net>      604-461-7532      <http://www.fireplug.net>
PGP Fingerprint: 28 E2 A0 15 99 62 9A 00  88 EC A3 EE 2D 1C 15 68