From 137028b4d7e50f71906c1656c27079eac5a1d085 Mon Sep 17 00:00:00 2001
From: Petr Baudis <pasky@suse.cz>
Date: Thu, 16 Jul 2009 10:10:10 -0700
Subject: Fix lock handling in memory hander of nscd.

The commit 20e498bd removes the pthread_mutex_rdlock() calls, but not the
corresponding pthread_mutex_unlock() calls. Also, the database lock is never
unlocked in one branch of the mempool_alloc() if.

I think unreproducible random assert(dh->usable) crashes in prune_cache() were
caused by this. But an easy way to make nscd threads hang with the broken
locking was.
---
 nscd/aicache.c       | 2 --
 nscd/grpcache.c      | 6 +-----
 nscd/hstcache.c      | 4 ----
 nscd/initgrcache.c   | 4 ----
 nscd/mem.c           | 6 +++---
 nscd/pwdcache.c      | 6 +-----
 nscd/servicescache.c | 4 ----
 7 files changed, 5 insertions(+), 27 deletions(-)

(limited to 'nscd')

diff --git a/nscd/aicache.c b/nscd/aicache.c
index 524c0a6..8dac48e 100644
--- a/nscd/aicache.c
+++ b/nscd/aicache.c
@@ -543,8 +543,6 @@ next_nip:
       (void) cache_add (req->type, key_copy, req->key_len, &dataset->head,
 			true, db, uid, he == NULL);
 
-      pthread_rwlock_unlock (&db->lock);
-
       /* Mark the old entry as obsolete.  */
       if (dh != NULL)
 	dh->usable = false;
diff --git a/nscd/grpcache.c b/nscd/grpcache.c
index 184d538..fc20084 100644
--- a/nscd/grpcache.c
+++ b/nscd/grpcache.c
@@ -146,8 +146,6 @@ cache_addgr (struct database_dyn *db, int fd, request_header *req,
 	      (void) cache_add (req->type, &dataset->strdata, req->key_len,
 				&dataset->head, true, db, owner, he == NULL);
 
-	      pthread_rwlock_unlock (&db->lock);
-
 	      /* Mark the old entry as obsolete.  */
 	      if (dh != NULL)
 		dh->usable = false;
@@ -367,12 +365,10 @@ cache_addgr (struct database_dyn *db, int fd, request_header *req,
 		(void) cache_add (GETGRBYGID, cp, key_offset, &dataset->head,
 				  false, db, owner, false);
 	    }
-
-	out:
-	  pthread_rwlock_unlock (&db->lock);
 	}
     }
 
+out:
   if (__builtin_expect (written != total, 0) && debug_level > 0)
     {
       char buf[256];
diff --git a/nscd/hstcache.c b/nscd/hstcache.c
index 51e2273..77ffcdf 100644
--- a/nscd/hstcache.c
+++ b/nscd/hstcache.c
@@ -153,8 +153,6 @@ cache_addhst (struct database_dyn *db, int fd, request_header *req,
 	      (void) cache_add (req->type, &dataset->strdata, req->key_len,
 				&dataset->head, true, db, owner, he == NULL);
 
-	      pthread_rwlock_unlock (&db->lock);
-
 	      /* Mark the old entry as obsolete.  */
 	      if (dh != NULL)
 		dh->usable = false;
@@ -404,8 +402,6 @@ cache_addhst (struct database_dyn *db, int fd, request_header *req,
 
 	  (void) cache_add (req->type, key_copy, req->key_len,
 			    &dataset->head, true, db, owner, he == NULL);
-
-	  pthread_rwlock_unlock (&db->lock);
 	}
     }
 
diff --git a/nscd/initgrcache.c b/nscd/initgrcache.c
index c33aaf3..f8d4742 100644
--- a/nscd/initgrcache.c
+++ b/nscd/initgrcache.c
@@ -230,8 +230,6 @@ addinitgroupsX (struct database_dyn *db, int fd, request_header *req,
 	      (void) cache_add (req->type, key_copy, req->key_len,
 				&dataset->head, true, db, uid, he == NULL);
 
-	      pthread_rwlock_unlock (&db->lock);
-
 	      /* Mark the old entry as obsolete.  */
 	      if (dh != NULL)
 		dh->usable = false;
@@ -388,8 +386,6 @@ addinitgroupsX (struct database_dyn *db, int fd, request_header *req,
 
 	  (void) cache_add (INITGROUPS, cp, req->key_len, &dataset->head, true,
 			    db, uid, he == NULL);
-
-	  pthread_rwlock_unlock (&db->lock);
 	}
     }
 
diff --git a/nscd/mem.c b/nscd/mem.c
index fcea6db..80ea951 100644
--- a/nscd/mem.c
+++ b/nscd/mem.c
@@ -566,9 +566,6 @@ mempool_alloc (struct database_dyn *db, size_t len, int data_alloc)
 	    }
 	}
 
-      if (data_alloc)
-	pthread_rwlock_unlock (&db->lock);
-
       if (! db->last_alloc_failed)
 	{
 	  dbg_log (_("no more memory for database '%s'"), dbnames[db - dbs]);
@@ -591,5 +588,8 @@ mempool_alloc (struct database_dyn *db, size_t len, int data_alloc)
 
   pthread_mutex_unlock (&db->memlock);
 
+  if (data_alloc)
+    pthread_rwlock_unlock (&db->lock);
+
   return res;
 }
diff --git a/nscd/pwdcache.c b/nscd/pwdcache.c
index 2338e7e..fc5b44e 100644
--- a/nscd/pwdcache.c
+++ b/nscd/pwdcache.c
@@ -153,8 +153,6 @@ cache_addpw (struct database_dyn *db, int fd, request_header *req,
 	      (void) cache_add (req->type, key_copy, req->key_len,
 				&dataset->head, true, db, owner, he == NULL);
 
-	      pthread_rwlock_unlock (&db->lock);
-
 	      /* Mark the old entry as obsolete.  */
 	      if (dh != NULL)
 		dh->usable = false;
@@ -362,12 +360,10 @@ cache_addpw (struct database_dyn *db, int fd, request_header *req,
 		(void) cache_add (GETPWBYUID, cp, key_offset, &dataset->head,
 				  false, db, owner, false);
 	    }
-
-	out:
-	  pthread_rwlock_unlock (&db->lock);
 	}
     }
 
+out:
   if (__builtin_expect (written != total, 0) && debug_level > 0)
     {
       char buf[256];
diff --git a/nscd/servicescache.c b/nscd/servicescache.c
index dc98d30..c965c97 100644
--- a/nscd/servicescache.c
+++ b/nscd/servicescache.c
@@ -136,8 +136,6 @@ cache_addserv (struct database_dyn *db, int fd, request_header *req,
 	      (void) cache_add (req->type, &dataset->strdata, req->key_len,
 				&dataset->head, true, db, owner, he == NULL);
 
-	      pthread_rwlock_unlock (&db->lock);
-
 	      /* Mark the old entry as obsolete.  */
 	      if (dh != NULL)
 		dh->usable = false;
@@ -317,8 +315,6 @@ cache_addserv (struct database_dyn *db, int fd, request_header *req,
 
 	  (void) cache_add (req->type, key_copy, req->key_len,
 			    &dataset->head, true, db, owner, he == NULL);
-
-	  pthread_rwlock_unlock (&db->lock);
 	}
     }
 
-- 
cgit v1.1