From patchwork Wed Jun 8 13:35:34 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Schwab X-Patchwork-Id: 12879 Received: (qmail 43243 invoked by alias); 8 Jun 2016 13:35:48 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 42842 invoked by uid 89); 8 Jun 2016 13:35:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=first, 3746, 374, 6, *db X-HELO: mx2.suse.de From: Andreas Schwab To: libc-alpha@sourceware.org Subject: [PATCH] Fix nscd assertion failure in gc (bug 19755) X-Yow: NOW, I'm supposed to SCRAMBLE two, and HOLD th' MAYO!! Date: Wed, 08 Jun 2016 15:35:34 +0200 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 If a GETxxBYyy request (for passwd or group) is running in parallel to an INVALIDATE request (for the same database) then in a particular order of events the garbage collector is not properly marking all used memory and fails an assertion: GETGRBYNAME (root) Haven't found "root" in group cache! add new entry "root" of type GETGRBYNAME for group to cache (first) handle_request: request received (Version = 2) from PID 7413 INVALIDATE (group) pruning group cache; time 9223372036854775807 considering GETGRBYNAME entry "root", timeout 1456763027 add new entry "0" of type GETGRBYGID for group to cache remove GETGRBYNAME entry "root" nscd: mem.c:403: gc: Assertion `next_data == &he_data[db->head->nentries]' failed. Here the first call to cache_add added the GETGRBYNAME entry, which is immediately marked for collection by prune_cache. Then the GETGRBYGID entry is added which shares the data packet with the first entry and therefore is marked as !first, while the marking look in prune_cache has already finished. When the garbage collector runs, it only considers references by entries marked as first, missing the reference by the secondary entry. The only way to fix that is to prevent prune_cache from running while the two related entries are added. [BZ #19755] * nscd/pwdcache.c (cache_addpw): Lock prune_run_lock while adding new entries in auto-propagate mode. * nscd/grpcache.c (cache_addgr): Likewise. --- nscd/grpcache.c | 13 ++++++++++++- nscd/pwdcache.c | 13 ++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/nscd/grpcache.c b/nscd/grpcache.c index 3831170..8b9b13d 100644 --- a/nscd/grpcache.c +++ b/nscd/grpcache.c @@ -205,10 +205,19 @@ cache_addgr (struct database_dyn *db, int fd, request_header *req, dataset = NULL; if (he == NULL) - dataset = (struct dataset *) mempool_alloc (db, total + n, 1); + { + /* Prevent an INVALIDATE request from pruning the data between + the two calls to cache_add. */ + if (db->propagate) + pthread_mutex_lock (&db->prune_run_lock); + dataset = (struct dataset *) mempool_alloc (db, total + n, 1); + } if (dataset == NULL) { + if (he == NULL && db->propagate) + pthread_mutex_unlock (&db->prune_run_lock); + /* We cannot permanently add the result in the moment. But we can provide the result as is. Store the data in some temporary memory. */ @@ -396,6 +405,8 @@ cache_addgr (struct database_dyn *db, int fd, request_header *req, out: pthread_rwlock_unlock (&db->lock); + if (he == NULL && db->propagate) + pthread_mutex_unlock (&db->prune_run_lock); } } diff --git a/nscd/pwdcache.c b/nscd/pwdcache.c index 6dd6746..5ef8485 100644 --- a/nscd/pwdcache.c +++ b/nscd/pwdcache.c @@ -198,10 +198,19 @@ cache_addpw (struct database_dyn *db, int fd, request_header *req, dataset = NULL; if (he == NULL) - dataset = (struct dataset *) mempool_alloc (db, total + n, 1); + { + /* Prevent an INVALIDATE request from pruning the data between + the two calls to cache_add. */ + if (db->propagate) + pthread_mutex_lock (&db->prune_run_lock); + dataset = (struct dataset *) mempool_alloc (db, total + n, 1); + } if (dataset == NULL) { + if (he == NULL && db->propagate) + pthread_mutex_unlock (&db->prune_run_lock); + /* We cannot permanently add the result in the moment. But we can provide the result as is. Store the data in some temporary memory. */ @@ -374,6 +383,8 @@ cache_addpw (struct database_dyn *db, int fd, request_header *req, out: pthread_rwlock_unlock (&db->lock); + if (he == NULL && db->propagate) + pthread_mutex_unlock (&db->prune_run_lock); } }