From bates at stat.wisc.edu  Tue Apr 27 15:13:35 2004
From: bates at stat.wisc.edu (Douglas Bates)
Date: Tue Apr 27 15:13:48 2004
Subject: [Rd] Segfault: .Call and classes with logical slots
In-Reply-To: <Pine.LNX.4.51.0404270757450.16528@artemis.imbe.med.uni-erlangen.de>
References: <Pine.LNX.4.51.0404261721500.13129@artemis.imbe.med.uni-erlangen.de>
	<408D7CC7.3E753469@research.bell-labs.com>
	<Pine.LNX.4.51.0404270757450.16528@artemis.imbe.med.uni-erlangen.de>
Message-ID: <6risflef7k.fsf@bates4.stat.wisc.edu>

torsten@hothorn.de writes:

> On Mon, 26 Apr 2004, John Chambers wrote:
> 
> > I think you need to PROTECT the vector you're putting in the slot as
> > well as the overall object.  At any rate, the problem goes away for me
> > with the revised version of dummy.c below.
> 
> yes, and it seems that PROTECT'ing the logical variable is sufficient
> while PROTECT'ing the class but not the logical variable causes a
> segfault again. I tried with numeric slots too: No problems.
> 
> > (Empirically, PROTECT'ing
> > the class definition didn't seem to be needed, but experience suggests
> > that too much protection  is better than too little.)
> 
> I tried to save (UN)PROTECT calls because of efficiency reasons. Anyway,
> this helps me a lot, thanks!

Perhaps this example is an indication that gctorture is too
aggressive.  I use constructions like

   PROTECT(ans = ...);

   SET_SLOT(ans, install("lgl"), allocVector(LGLSXP,1));
   LOGICAL(GET_SLOT(ans, install("lgl")))[0] = TRUE;

in many places in my code, having been assured by a usually reliable
source (Luke) that SET_SLOT applied to a freshly allocated vector
would be atomic with respect to garbage collection.  That is, under
the usual conditions there would be no chance of a garbage
collection being triggered between the allocVector and SET_SLOT
operations.  It may be that gctorture is causing a garbage collection
at a place where it otherwise could not occur and the additional
(UN)PROTECT are redundant except when gctorture is active.

In trying to avoid (UN)PROTECT calls I'm not as concerned about
efficiency as I am about clarity of the code.  I would prefer not to
clutter the code with (UN)PROTECT calls if they are known to be
redundant.

At the time we discussed this Luke suggested that we document a set of
C calls that are atomic with respect to garbage collection.  I think
this would be a good idea but I suspect that no one has the time to do
it right now.

> 
> Torsten
> 
> >
> 
> 
> 
> 
> > #include <Rdefines.h>
> >
> > SEXP foo() {
> >
> >     SEXP ans, cl, el;
> >
> >     PROTECT(cl = MAKE_CLASS("test"));
> >     PROTECT(ans = NEW_OBJECT(cl));
> >     PROTECT(el = allocVector(LGLSXP, 1));
> >     SET_SLOT(ans, install("lgl"), el);
> >     LOGICAL(GET_SLOT(ans, install("lgl")))[0] = TRUE;
> >     UNPROTECT(3);
> >     return(ans);
> > }
> >
> >
> > Torsten Hothorn wrote:
> > >
> > > Hi,
> > >
> > > the following example aiming at a class containing a logical slot
> > > segfaults under R-1.9.0 when `gctorture(on = TRUE)' is used:
> > >
> > > Code code (dummy.c):
> > >
> > > #include <Rdefines.h>
> > >
> > > SEXP foo() {
> > >
> > >     SEXP ans;
> > >
> > >     PROTECT(ans = NEW_OBJECT(MAKE_CLASS("test")));
> > >     SET_SLOT(ans, install("lgl"), allocVector(LGLSXP, 1));
> > >     LOGICAL(GET_SLOT(ans, install("lgl")))[0] = TRUE;
> > >     UNPROTECT(1);
> > >     return(ans);
> > > }
> > >
> > > R code (dummy.R):
> > >
> > > dyn.load("dummy.so")
> > >
> > > setClass("test", representation = representation(lgl = "logical"))
> > >
> > > a = .Call("foo")
> > > a # OK
> > >
> > > gctorture(on = TRUE)
> > > a = .Call("foo")
> > > gctorture(on = FALSE)
> > > a # segfault
> > >
> > > which gives
> > >
> > > R>
> > > R>
> > > R> dyn.load("dummy.so")
> > > R>
> > > R> setClass("test", representation = representation(lgl = "logical"))
> > > [1] "test"
> > > R>
> > > R> a = .Call("foo")
> > > R> a
> > > An object of class "test"
> > > Slot "lgl":
> > > [1] TRUE
> > >
> > > R>
> > > R> gctorture(on = TRUE)
> > > R> a = .Call("foo")
> > > R> gctorture(on = FALSE)
> > > Segmentation fault
> > >
> > > Best,
> > >
> > > Torsten
> > >
> > > R> version
> > >          _
> > > platform i686-pc-linux-gnu
> > > arch     i686
> > > os       linux-gnu
> > > system   i686, linux-gnu
> > > status
> > > major    1
> > > minor    9.0
> > > year     2004
> > > month    04
> > > day      12
> > > language R
> > > R>
> > >
> > >  _______________________________________________________________________
> > > |                                                                       |
> > > |       Dr. rer. nat. Torsten Hothorn                                   |
> > > |       Institut fuer Medizininformatik, Biometrie und Epidemiologie    |
> > > |       Waldstrasse 6, D-91054 Erlangen, Deutschland                    |
> > > |       Tel: ++49-9131-85-22707 (dienstl.)                              |
> > > |       Fax: ++49-9131-85-25740                                         |
> > > |       Email:  Torsten.Hothorn@rzmail.uni-erlangen.de                  |
> > > |               PLEASE send emails cc to torsten@hothorn.de             |
> > > |       Web: http://www.imbe.med.uni-erlangen.de/~hothorn               |
> > > |_______________________________________________________________________|
> > >
> > > ______________________________________________
> > > R-devel@stat.math.ethz.ch mailing list
> > > https://www.stat.math.ethz.ch/mailman/listinfo/r-devel
> >
> > --
> > John M. Chambers                  jmc@bell-labs.com
> > Bell Labs, Lucent Technologies    office: (908)582-2681
> > 700 Mountain Avenue, Room 2C-282  fax:    (908)582-3340
> > Murray Hill, NJ  07974            web: http://www.cs.bell-labs.com/~jmc
> >
> 
> ______________________________________________
> R-devel@stat.math.ethz.ch mailing list
> https://www.stat.math.ethz.ch/mailman/listinfo/r-devel

-- 
Douglas Bates                            bates@stat.wisc.edu
Statistics Department                    608/262-2598
University of Wisconsin - Madison        http://www.stat.wisc.edu/~bates/


[Rd] Segfault: .Call and classes with logical slots
Douglas Bates