[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Guile-commits] 01/01: Fix race between SMOB marking and finalization
From: |
Andy Wingo |
Subject: |
[Guile-commits] 01/01: Fix race between SMOB marking and finalization |
Date: |
Thu, 23 Jun 2016 10:13:33 +0000 (UTC) |
wingo pushed a commit to branch master
in repository guile.
commit c01a2a757e3c59727bdfa8d77568bf42525fbe05
Author: Andy Wingo <address@hidden>
Date: Thu Jun 23 11:47:42 2016 +0200
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(-)
diff --git a/libguile/smob.c b/libguile/smob.c
index 6a97caa..43ea613 100644
--- a/libguile/smob.c
+++ b/libguile/smob.c
@@ -372,20 +372,43 @@ scm_gc_mark (SCM o)
}
+static void*
+clear_smobnum (void *ptr)
+{
+ SCM smob;
+ scm_t_bits smobnum;
+
+ smob = SCM_PACK_POINTER (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 = SCM_PACK_POINTER (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);
}
@@ -460,6 +483,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);
@@ -483,6 +507,9 @@ scm_smob_prehistory ()
scm_smobs[i].apply = 0;
scm_smobs[i].apply_trampoline = 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 5138b15..524a144 100644
--- a/test-suite/standalone/Makefile.am
+++ b/test-suite/standalone/Makefile.am
@@ -286,6 +286,12 @@ 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
+
check_SCRIPTS += test-stack-overflow
TESTS += test-stack-overflow
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 <config.h>
+#endif
+
+#undef NDEBUG
+
+#include <libguile.h>
+#include <assert.h>
+
+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;
+}