GNU bug report logs - #19883
Smob's mark_smob has become unreliable in Guile 2.x

Previous Next

Full log

Message #56 received at 19883 <at> debbugs.gnu.org (full text, mbox):

From: Andy Wingo <wingo <at> pobox.com>
To: David Kastrup <dak <at> gnu.org>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 19883 <at> debbugs.gnu.org
Subject: Re: bug#19883: Correction for backtrace
Date: Thu, 23 Jun 2016 11:50:48 +0200

On Thu 26 Feb 2015 16:30, David Kastrup <dak <at> gnu.org> 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?

Andy

commit db30120fc3a1727d8f221cbb014314f2babf841e
Author: Andy Wingo <wingo <at> pobox.com>
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.

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 (&current_mark_stack_pointer, NULL);
   scm_i_pthread_key_create (&current_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 ();
 }
 
 /*

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.