From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Andy Wingo Newsgroups: gmane.lisp.guile.bugs Subject: bug#19883: Correction for backtrace Date: Thu, 23 Jun 2016 12:36:34 +0200 Message-ID: <87bn2sp4y5.fsf@pobox.com> References: <87twyln70f.fsf@fencepost.gnu.org> <873865n2rr.fsf@fencepost.gnu.org> <87twy91vtc.fsf@gnu.org> <878ufld1iw.fsf@fencepost.gnu.org> <87k2z4dhx7.fsf@gnu.org> <874mq8dfb3.fsf@fencepost.gnu.org> <87fv9sahxx.fsf@gnu.org> <87wq34bsh7.fsf@fencepost.gnu.org> <87fus4p72f.fsf@pobox.com> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1466678254 7955 80.91.229.3 (23 Jun 2016 10:37:34 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 23 Jun 2016 10:37:34 +0000 (UTC) Cc: 19883-done@debbugs.gnu.org, David Kastrup To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-X-From: bug-guile-bounces+guile-bugs=m.gmane.org@gnu.org Thu Jun 23 12:37:23 2016 Return-path: Envelope-to: guile-bugs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1bG20c-0005AE-LL for guile-bugs@m.gmane.org; Thu, 23 Jun 2016 12:37:18 +0200 Original-Received: from localhost ([::1]:35759 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bG20W-0004gQ-R2 for guile-bugs@m.gmane.org; Thu, 23 Jun 2016 06:37:12 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:51526) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bG20O-0004dL-PZ for bug-guile@gnu.org; Thu, 23 Jun 2016 06:37:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bG20M-0006A9-3W for bug-guile@gnu.org; Thu, 23 Jun 2016 06:37:03 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:39485) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bG20L-0006A3-Vm for bug-guile@gnu.org; Thu, 23 Jun 2016 06:37:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bG20L-0002eI-O7 for bug-guile@gnu.org; Thu, 23 Jun 2016 06:37:01 -0400 Resent-From: Andy Wingo Original-Sender: "Debbugs-submit" Resent-To: bug-guile@gnu.org Resent-Date: Thu, 23 Jun 2016 10:37:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: cc-closed 19883 X-GNU-PR-Package: guile X-GNU-PR-Keywords: Mail-Followup-To: 19883@debbugs.gnu.org, wingo@pobox.com, dak@gnu.org Original-Received: via spool by 19883-done@debbugs.gnu.org id=D19883.146667820810159 (code D ref 19883); Thu, 23 Jun 2016 10:37:01 +0000 Original-Received: (at 19883-done) by debbugs.gnu.org; 23 Jun 2016 10:36:48 +0000 Original-Received: from localhost ([127.0.0.1]:51822 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bG208-0002dn-Ar for submit@debbugs.gnu.org; Thu, 23 Jun 2016 06:36:48 -0400 Original-Received: from pb-sasl2.pobox.com ([64.147.108.67]:55557 helo=sasl.smtp.pobox.com) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bG205-0002de-Vq for 19883-done@debbugs.gnu.org; Thu, 23 Jun 2016 06:36:46 -0400 Original-Received: from sasl.smtp.pobox.com (unknown [127.0.0.1]) by pb-sasl2.pobox.com (Postfix) with ESMTP id 18A6F21E98; Thu, 23 Jun 2016 06:36:43 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; s=sasl; bh=MGi0j+5NvrqOAKMu/dweohHr9d8=; b=gJilzZ Loc+MK5nGlO2Sit5hO0TtpYEWQ8i2RoUcaciTTk8mKEd5WXpdln66CE5Mw4m2+hU v74KGKejq3yyD6GfjMGa7aQu1naBANz7QrJo0UO4VCO7RPzPZTyf2NFPAhjS6192 y3nyKBSxpjsz0g8sCnVm6X1QR5egn30psVgx4= DomainKey-Signature: a=rsa-sha1; c=nofws; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; q=dns; s=sasl; b=vA4l92uGjJubE3EsQ2echO7n9bCJDVuu apgvRDBH8itQcP7zzSKnrzZ9Bb4+B76JWgQRP7N0meH9I+8+9hi7U5hBQ77SFAVk ufZEX1+fZMIcLDIRO3DP/4k/lsslOHDaTHahHSS7qtS3+1vfJk9hKdJYEmv8hnmz bcYStRzKNrc= Original-Received: from pb-sasl2.nyi.icgroup.com (unknown [127.0.0.1]) by pb-sasl2.pobox.com (Postfix) with ESMTP id 10D5421E97; Thu, 23 Jun 2016 06:36:43 -0400 (EDT) Original-Received: from clucks (unknown [88.160.190.192]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by pb-sasl2.pobox.com (Postfix) with ESMTPSA id 14BB021E95; Thu, 23 Jun 2016 06:36:41 -0400 (EDT) In-Reply-To: <87fus4p72f.fsf@pobox.com> (Andy Wingo's message of "Thu, 23 Jun 2016 11:50:48 +0200") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) X-Pobox-Relay-ID: 60073636-392E-11E6-AB61-28A6F1301B6D-02397024!pb-sasl2.pobox.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-guile@gnu.org List-Id: "Bug reports for GUILE, GNU's Ubiquitous Extension Language" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guile-bounces+guile-bugs=m.gmane.org@gnu.org Original-Sender: "bug-guile" Xref: news.gmane.org gmane.lisp.guile.bugs:8152 Archived-At: --=-=-= Content-Type: text/plain On Thu 23 Jun 2016 11:50, Andy Wingo writes: > On Thu 26 Feb 2015 16:30, David Kastrup writes: > >> Try ./test 2 2000 200 > > I can reproduce the crash with your test case, thanks :) The patch below > fixes the bug for me. WDYT Ludovic? Here's a patch with a test case. I'm going to apply as it seems to be obviously the right thing and the test case does reproduce what I think is the bug (racing mark and finalize procedures, even if it's only happening in one thread, finalizers and mark procedures do introduce concurrency). We trigger the concurrency in a simple way, via allocation in the finalizer. The patch does fix the original test. GC could happen due to another thread of course. I'm actually not sure where the concurrency is coming from in David's test though :/ I'm very interested in any feedback you might have! Andy --=-=-= Content-Type: text/plain Content-Disposition: inline; filename=0001-Fix-race-between-SMOB-marking-and-finalization.patch >From 8dff3af087c6eaa83ae0d72aa8b22aef5c65d65d Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Thu, 23 Jun 2016 11:47:42 +0200 Subject: [PATCH] Fix race between SMOB marking and finalization * libguile/smob.c (clear_smobnum): New helper. (finalize_smob): Re-set the smobnum to the "finalized smob" type before finalizing. Fixes #19883. (scm_smob_prehistory): Pre-register a "finalized smob" type, which has no mark procedure. * test-suite/standalone/test-smob-mark-race.c: New file. * test-suite/standalone/Makefile.am: Arrange to build and run the new test. --- libguile/smob.c | 33 +++++++++++++-- test-suite/standalone/Makefile.am | 6 +++ test-suite/standalone/test-smob-mark-race.c | 65 +++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 test-suite/standalone/test-smob-mark-race.c diff --git a/libguile/smob.c b/libguile/smob.c index 90849a8..ed9d91a 100644 --- a/libguile/smob.c +++ b/libguile/smob.c @@ -374,20 +374,43 @@ scm_gc_mark (SCM o) } +static void* +clear_smobnum (void *ptr) +{ + SCM smob; + scm_t_bits smobnum; + + smob = PTR2SCM (ptr); + + smobnum = SCM_SMOBNUM (smob); + /* Frob the object's type in place, re-setting it to be the "finalized + smob" type. This will prevent other routines from accessing its + internals in a way that assumes that the smob data is valid. This + is notably the case for SMOB's own "mark" procedure, if any; as the + finalizer runs without the alloc lock, it's possible for a GC to + occur while it's running, in which case the object is alive and yet + its data is invalid. */ + SCM_SET_SMOB_DATA_0 (smob, SCM_SMOB_DATA_0 (smob) & ~(scm_t_bits) 0xff00); + + return (void *) smobnum; +} + /* Finalize SMOB by calling its SMOB type's free function, if any. */ static void finalize_smob (void *ptr, void *data) { SCM smob; + scm_t_bits smobnum; size_t (* free_smob) (SCM); smob = PTR2SCM (ptr); + smobnum = (scm_t_bits) GC_call_with_alloc_lock (clear_smobnum, ptr); + #if 0 - printf ("finalizing SMOB %p (smobnum: %u)\n", - ptr, SCM_SMOBNUM (smob)); + printf ("finalizing SMOB %p (smobnum: %u)\n", ptr, smobnum); #endif - free_smob = scm_smobs[SCM_SMOBNUM (smob)].free; + free_smob = scm_smobs[smobnum].free; if (free_smob) free_smob (smob); } @@ -470,6 +493,7 @@ void scm_smob_prehistory () { long i; + scm_t_bits finalized_smob_tc16; scm_i_pthread_key_create (¤t_mark_stack_pointer, NULL); scm_i_pthread_key_create (¤t_mark_stack_limit, NULL); @@ -493,6 +517,9 @@ scm_smob_prehistory () scm_smobs[i].apply = 0; scm_smobs[i].apply_trampoline_objcode = SCM_BOOL_F; } + + finalized_smob_tc16 = scm_make_smob_type ("finalized smob", 0); + if (SCM_TC2SMOBNUM (finalized_smob_tc16) != 0) abort (); } /* diff --git a/test-suite/standalone/Makefile.am b/test-suite/standalone/Makefile.am index 712418a..aec7418 100644 --- a/test-suite/standalone/Makefile.am +++ b/test-suite/standalone/Makefile.am @@ -275,4 +275,10 @@ test_smob_mark_LDADD = $(LIBGUILE_LDADD) check_PROGRAMS += test-smob-mark TESTS += test-smob-mark +test_smob_mark_race_SOURCES = test-smob-mark-race.c +test_smob_mark_race_CFLAGS = ${test_cflags} +test_smob_mark_race_LDADD = $(LIBGUILE_LDADD) +check_PROGRAMS += test-smob-mark-race +TESTS += test-smob-mark-race + EXTRA_DIST += ${check_SCRIPTS} diff --git a/test-suite/standalone/test-smob-mark-race.c b/test-suite/standalone/test-smob-mark-race.c new file mode 100644 index 0000000..eca0325 --- /dev/null +++ b/test-suite/standalone/test-smob-mark-race.c @@ -0,0 +1,65 @@ +/* Copyright (C) 2016 Free Software Foundation, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public License + * as published by the Free Software Foundation; either version 3 of + * the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301 USA + */ + +#if HAVE_CONFIG_H +#include +#endif + +#undef NDEBUG + +#include +#include + +static SCM +mark_smob (SCM smob) +{ + assert (SCM_SMOB_DATA (smob) == 1); + return SCM_BOOL_F; +} + +static size_t +finalize_smob (SCM smob) +{ + assert (SCM_SMOB_DATA (smob) == 1); + SCM_SET_SMOB_DATA (smob, 0); + /* Allocate a bit in the hopes of triggering a new GC, making the + marker race with the finalizer. */ + scm_cons (SCM_BOOL_F, SCM_BOOL_F); + return 0; +} + +static void +tests (void *data, int argc, char **argv) +{ + scm_t_bits tc16; + int i; + + tc16 = scm_make_smob_type ("smob with finalizer", 0); + scm_set_smob_mark (tc16, mark_smob); + scm_set_smob_free (tc16, finalize_smob); + + for (i = 0; i < 1000 * 1000; i++) + scm_new_smob (tc16, 1); +} + +int +main (int argc, char *argv[]) +{ + scm_boot_guile (argc, argv, tests, NULL); + return 0; +} -- 2.8.3 --=-=-=--