Skip to content

Commit

Permalink
Fix lock handling in memory hander of nscd.
Browse files Browse the repository at this point in the history
The commit 20e498b 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.
  • Loading branch information
Petr Baudis authored and Ulrich Drepper committed Jul 16, 2009
1 parent 50158f9 commit 137028b
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 27 deletions.
11 changes: 11 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,10 +1,21 @@
2009-07-16 Petr Baudis <pasky@suse.cz>

* nscd/mem.c (mempool_alloc): Fix unlock missing in the else branch.
* nscd/aicache.c: Remove bogus db->lock unlock.
* nscd/grpcache.c: Likewise.
* nscd/hstcache.c: Likewise.
* nscd/initgrcache.c: Likewise.
* nscd/pwdcache.c: Likewise.
* nscd/servicescache.c: Likewise.

2009-07-16 Ulrich Drepper <drepper@redhat.com>

* nscd/cache.c (cache_add): Use atomic_compare_and_exchange_bool_rel
instead of atomic_compare_and_exchange_bool_acq to ensure pointer
is written before the list head update.
Patch by Andreas Schwab <aschwab@redhat.com>.

2009-07-16 Ulrich Drepper <drepper@redhat.com>
Jakub Jelinek <jakub@redhat.com>

* malloc/malloc.c [ATOMIC_FASTBINS] (_int_free): Make check for
Expand Down
2 changes: 0 additions & 2 deletions nscd/aicache.c
Original file line number Diff line number Diff line change
Expand Up @@ -543,8 +543,6 @@ addhstaiX (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;
Expand Down
6 changes: 1 addition & 5 deletions nscd/grpcache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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];
Expand Down
4 changes: 0 additions & 4 deletions nscd/hstcache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
4 changes: 0 additions & 4 deletions nscd/initgrcache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down
6 changes: 3 additions & 3 deletions nscd/mem.c
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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;
}
6 changes: 1 addition & 5 deletions nscd/pwdcache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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];
Expand Down
4 changes: 0 additions & 4 deletions nscd/servicescache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 137028b

Please sign in to comment.