r20690: fix a bug that causes smbd to 'hang' intermittently.

The problem occurs like this:

  1) running smbd as a domain member without winbindd

  2) client1 connects, during auth smbd-1 calls update_trustdom_cache()

  3) smbd-1 takes the trustdom cache timestamp lock, then starts
     enumerate_domain_trusts

  4) enumerate_domain_trusts hangs for some unknown reason

  5) other clients connect, all block waiting for read lock on trustdom
     cache

  6) samba is now hung

The problem is the lock, and really its just trying to avoid a race
where the cure is worse than the problem. A race in updating the
trutdom cache is not a big issue. So I've just removed the lock.

It is still an open question why enumerate_domain_trusts() can
hang. Unfortunately I've not in a position to get a sniff at the site
that is affected. I suspect a full fix will involve ensuring that all
the rpc code paths have appropriate timeouts.
(This used to be commit ab8d41053347a5b342ed5b59a0b0dd4983ca91e6)

1 files changed, 13 insertions, 32 deletions

diff --git a/source3/libsmb/trustdom_cache.c b/source3/libsmb/trustdom_cache.c
index fa35f8d423..dc0b5010a2 100644
--- a/source3/libsmb/trustdom_cache.c
+++ b/source3/libsmb/trustdom_cache.c
@@ -250,24 +250,6 @@ BOOL trustdom_cache_store_timestamp( uint32 t, time_t timeout )
 }
 
 
-/*******************************************************************
- lock the timestamp entry in the trustdom_cache
-*******************************************************************/
-
-BOOL trustdom_cache_lock_timestamp( void )
-{
-	return gencache_lock_entry( TDOMTSKEY ) != -1;
-}
-
-/*******************************************************************
- unlock the timestamp entry in the trustdom_cache
-*******************************************************************/
-
-void trustdom_cache_unlock_timestamp( void )
-{
-	gencache_unlock_entry( TDOMTSKEY );
-}
-
 /**
  * Delete single trustdom entry. Look at the
  * gencache_iterate definition.
@@ -314,8 +296,7 @@ void update_trustdom_cache( void )
 	time_t now = time(NULL);
 	int i;
 	
-	/* get the timestamp.  We have to initialise it if the last timestamp == 0 */
-	
+	/* get the timestamp.  We have to initialise it if the last timestamp == 0 */	
 	if ( (last_check = trustdom_cache_fetch_timestamp()) == 0 ) 
 		trustdom_cache_store_timestamp(0, now+TRUSTDOM_UPDATE_INTERVAL);
 
@@ -325,11 +306,12 @@ void update_trustdom_cache( void )
 		DEBUG(10,("update_trustdom_cache: not time to update trustdom_cache yet\n"));
 		return;
 	}
+
+	/* note that we don't lock the timestamp. This prevents this
+	   smbd from blocking all other smbd daemons while we
+	   enumerate the trusted domains */
+	trustdom_cache_store_timestamp(now, now+TRUSTDOM_UPDATE_INTERVAL);
 		
-	/* lock the timestamp */
-	if ( !trustdom_cache_lock_timestamp() )
-		return;
-	
 	if ( !(mem_ctx = talloc_init("update_trustdom_cache")) ) {
 		DEBUG(0,("update_trustdom_cache: talloc_init() failed!\n"));
 		goto done;
@@ -338,20 +320,19 @@ void update_trustdom_cache( void )
 	/* get the domains and store them */
 	
 	if ( enumerate_domain_trusts(mem_ctx, lp_workgroup(), &domain_names, 
-		&num_domains, &dom_sids) ) 
-	{
+		&num_domains, &dom_sids)) {
 		for ( i=0; i<num_domains; i++ ) {
 			trustdom_cache_store( domain_names[i], NULL, &dom_sids[i], 
 				now+TRUSTDOM_UPDATE_INTERVAL);
-		}
-		
-		trustdom_cache_store_timestamp( now, now+TRUSTDOM_UPDATE_INTERVAL );
+		}		
+	} else {
+		/* we failed to fetch the list of trusted domains - restore the old
+		   timestamp */
+		trustdom_cache_store_timestamp(last_check, 
+					       last_check+TRUSTDOM_UPDATE_INTERVAL);
 	}
 
 done:	
-	/* unlock and we're done */
-	trustdom_cache_unlock_timestamp();
-	
 	talloc_destroy( mem_ctx );
 	
 	return;