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

[John Chambers]

I was about to comment with some of the same points Luke makes here. 
It's hard to see how gctorture could be less aggressive and still
guarantee to find problems.  Yes, some of the problems look unlikely,
but that's partly what makes them so insidious.

Two additional small points, one of detail, one about style.

1. A slight expansion on SET_SLOT.  There are two situations:  ordinary
slots and the ".Data" slot, a way to set the "data part" of an object.

The allocation for ordinary slots is trivial and maybe there is a way to
avoid it.  If the name argument is a symbol (as it usually is), SET_SLOT
allocates a corresponding character vector, because that's what
set_attrib wants.  Seems like each symbol could have a corresponding
object of this form (maybe there is one already?) to avoid allocation in
this case.

The .Data case involves much more code.  In this special case, should
SET_SLOT PROTECT the value argument?  or for that matter, would it be a
serious overhead for SET_SLOT to PROTECT the value argument always?

2.  On the other hand, this style of example might be characterized as
"hand compiling" S language code.  Many of us have had the experience
that such hand compiling is very error prone.  I know it's sometimes
strongly motivated, but it's likely to be an unpleasant experience.

If it's at all possible, the long-standing advice applies:  Try to
pre-allocate the data needed and keep the C code to dealing with
existing objects (e.g., DATAPTR() pointers); in particular, so the heavy
C code uses only pointers to data, not R objects.

And hopefully real compiling will eventually relieve us of some of the
need.

John

Luke Tierney wrote:
> 
> On 27 Apr 2004, Douglas Bates wrote:
> 
> > torsten at 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
> 
> It can't be--it only forces gc in places that _could_ result in a gc
> withut gctorture; it will not result in a gc in places that otherwise
> could not.  The result may be a gc in places that otherwise would be
> very unlikely to cause one but not in places that could not.
> 
> >    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.
> 
> There are two different scenarios.  Some things are guaranteed not to
> allocate, for example low level operations like SET_VECTOR_ELT.
> Others, like setAttrib do allocate, but when they do they protect
> (some of) their arguments.  So code that uses setAttrib does not need
> to protect the arguments to the call (at least the value one--I'd have
> to double check the others).  Other variables that are alive before
> and after the setAttrib call will need to be protected.
> 
> Since slots are stored in attributes we can at most hope for the
> second behavior.  But we do not have it.  SET_SLOT is a macro that
> expands to R_do_slot_assign, which starts out as
> 
> SEXP R_do_slot_assign(SEXP obj, SEXP name, SEXP value) {
>     SEXP input = name; int nprotect = 0;
>     if(isSymbol(name) ) {
>         input = PROTECT(allocVector(STRSXP, 1)); nprotect++; /******/
>         SET_STRING_ELT(input, 0, PRINTNAME(name));
>     }
>     else if(!(isString(name) && LENGTH(name) == 1))
>         error("invalid type or length for slot name");
>     ...
> 
> The actual assignment uses setAttrib, which does operate in a way that
> protects the value being assigned, but the unprotected allocation at
> /******/ happens before we get there.
> 
> So unless we modify SET_SLOT to protect the value argument (and the
> others as well to be safe), the value needs to be protected (as do any
> other objects that might be needed after the call).
> 
> Best,
> 
> luke
> 
> --
> Luke Tierney
> University of Iowa                  Phone:             319-335-3386
> Department of Statistics and        Fax:               319-335-3017
>    Actuarial Science
> 241 Schaeffer Hall                  email:      luke at stat.uiowa.edu
> Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

-- 
John M. Chambers                  jmc at 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