From patchwork Tue Dec 14 05:16:27 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 75468 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 5EFDEB7082 for ; Tue, 14 Dec 2010 16:16:50 +1100 (EST) Received: (qmail 25515 invoked by alias); 14 Dec 2010 05:16:47 -0000 Received: (qmail 25501 invoked by uid 22791); 14 Dec 2010 05:16:44 -0000 X-SWARE-Spam-Status: No, hits=-4.7 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (74.125.121.35) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 14 Dec 2010 05:16:35 +0000 Received: from wpaz33.hot.corp.google.com (wpaz33.hot.corp.google.com [172.24.198.97]) by smtp-out.google.com with ESMTP id oBE5GWGN020933 for ; Mon, 13 Dec 2010 21:16:32 -0800 Received: from yws5 (yws5.prod.google.com [10.192.19.5]) by wpaz33.hot.corp.google.com with ESMTP id oBE5GU1h028604 (version=TLSv1/SSLv3 cipher=RC4-MD5 bits=128 verify=NOT) for ; Mon, 13 Dec 2010 21:16:31 -0800 Received: by yws5 with SMTP id 5so130297yws.29 for ; Mon, 13 Dec 2010 21:16:30 -0800 (PST) Received: by 10.100.255.20 with SMTP id c20mr3252535ani.195.1292303790754; Mon, 13 Dec 2010 21:16:30 -0800 (PST) Received: from coign.google.com (adsl-71-133-8-30.dsl.pltn13.pacbell.net [71.133.8.30]) by mx.google.com with ESMTPS id f10sm1008052anh.25.2010.12.13.21.16.29 (version=TLSv1/SSLv3 cipher=RC4-MD5); Mon, 13 Dec 2010 21:16:30 -0800 (PST) From: Ian Lance Taylor To: gcc-patches@gcc.gnu.org, gofrontend-dev@googlegroups.com Subject: libgo patch committed: Fix locking for release and allocate cache Date: Mon, 13 Dec 2010 21:16:27 -0800 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 X-System-Of-Record: true X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org In the last libgo update I introduced a bug. I changed the code to not hold the thread_ids lock while releasing the cache. That was a mistake, because a garbage collection could occur after the thread_ids lock was released but before the cache was released. That could cause the thread's m structure to be released, which could in turn cause a null pointer dereference when updating statistics. Conversely, it is no longer necessary to hold the thread_ids lock while allocating the cache. Previously that served as a lock for runtime_mheap.cachealloc, but that is no longer necessary as the code now uses the runtime_mheap lock instead. This patch implements both changes. Committed to mainline. Ian diff -r 3ac32bd87c72 libgo/runtime/go-go.c --- a/libgo/runtime/go-go.c Thu Dec 09 15:54:35 2010 -0800 +++ b/libgo/runtime/go-go.c Mon Dec 13 21:11:03 2010 -0800 @@ -107,11 +107,11 @@ if (list_entry->next != NULL) list_entry->next->prev = list_entry->prev; + runtime_MCache_ReleaseAll (mcache); + i = pthread_mutex_unlock (&__go_thread_ids_lock); __go_assert (i == 0); - runtime_MCache_ReleaseAll (mcache); - runtime_lock (&runtime_mheap); mstats.heap_alloc += mcache->local_alloc; mstats.heap_objects += mcache->local_objects; @@ -225,14 +225,13 @@ newm->list_entry = list_entry; + newm->mcache = runtime_allocmcache (); + /* Add the thread to the list of all threads, marked as tentative since it is not yet ready to go. */ i = pthread_mutex_lock (&__go_thread_ids_lock); __go_assert (i == 0); - /* We use __go_thread_ids_lock as a lock for mheap.cachealloc. */ - newm->mcache = runtime_allocmcache (); - if (__go_all_thread_ids != NULL) __go_all_thread_ids->prev = list_entry; list_entry->next = __go_all_thread_ids;