From hpages at fhcrc.org  Wed Jan 16 20:02:46 2008
From: hpages at fhcrc.org (Herve Pages)
Date: Wed, 16 Jan 2008 11:02:46 -0800
Subject: [Rd] Pb with defineVar() example in the "Writing R Extensions"
 manual
In-Reply-To: <478E48CD.3000602@biostat.ku.dk>
References: <478D549E.40406@fhcrc.org> <478DD547.5080608@biostat.ku.dk>
	<478E4577.8040402@fhcrc.org> <478E48CD.3000602@biostat.ku.dk>
Message-ID: <478E54D6.5040301@fhcrc.org>

Peter Dalgaard wrote:
> Herve Pages wrote:
>> Hi Peter,
>>
>> Peter Dalgaard wrote:
>>   
>>> Herve Pages wrote:
>>>     
>>>> Hi,
>>>>
>>>> I'm wondering if this code from the "Writing R Extensions" manual
>>>> is really safe:
>>>>
>>>>      SEXP mkans(double x)
>>>>      {
>>>>          SEXP ans;
>>>>          PROTECT(ans = allocVector(REALSXP, 1));
>>>>          REAL(ans)[0] = x;
>>>>          UNPROTECT(1);
>>>>          return ans;
>>>>      }
>>>>
>>>>      double feval(double x, SEXP f, SEXP rho)
>>>>      {
>>>>          defineVar(install("x"), mkans(x), rho);
>>>>          return(REAL(eval(f, rho))[0]);
>>>>      }
>>>>
>>>> In C, the order in which function arguments are evaluated before the
>>>> function itself is called is undefined. Hence there is no guarantee
>>>> that install("x") will be evaluated before mkans(x). What happens if
>>>> mkans(x) is evaluated first? Then install("x") will be called and
>>>> eventually trigger garbage collection while the SEXP returned by
>>>> mkans(x) is still unprotected.
>>>>
>>>> I'm asking because I'm getting all sorts of problems with
>>>>
>>>>   defineVar(install(somekey), mkans(x), rho);
>>>>
>>>> In my code this line is inside a big loop (hundred of thousands of
>>>> iterations) so I end up with a lot of symbols in the rho environment.
>>>>
>>>> The problems I've seen are hard to reproduce: sometimes it's a segfault,
>>>> sometimes a "cons memory exhausted" error, or sometimes everything looks
>>>> fine except that, later, when I retrieve values from the rho environment
>>>> with findVar(), some of them are altered!
>>>>
>>>> But if I replace the above line by:
>>>>
>>>>   PROTECT(ans = mkans(x));
>>>>   defineVar(install(somekey), ans, rho);
>>>>   UNPROTECT(1);
>>>>
>>>> then everything works fine :-)
>>>>
>>>>   
>>>>       
>>> Sounds like you are right. You don't really have the "smoking gun", but
>>> it doesn't seem to be worth trying to catch the actual bug in action
>>> with hardware watchpoints and whatnot.
>>>
>>> The opposite fix should work too (does it?):
>>>
>>> { SEXP sym = install(somekey) ; defineVar(sym, mkans(x), rho);}
>>>     
>> So now you are protected against install(somekey) eventually triggering
>> garbage collection but you are still not protected against defineVar() itself
>> triggering garbage collection. Maybe defineVar() does not do that, and will
>> never do it, but isn't it risky to rely on this kind of assumption?
>>
>> Thanks!
>> H.
>>   
> That's not the problem you raised (argument evaluation order), but
> there's a CONS inside defineVar, and as far as I can see, it doesn't
> protect its arguments, so you could well be right.

This problem is related to my original problem since it would cause the same
disaster: garbage collection on my unprotected SEXP.
The more general problem I'm facing is to know whether or not it is safe to
use a function like mkans() (that returns an unprotected SEXP) like this:

  SET_ELEMENT(ans, 0, mkans(x));

In the case of SET_ELEMENT() or SET_STRING_ELT() it seems to be safe. For
example I've seen this

  SET_STRING_ELT(ans, 0, mkChar(buf));

in many places. So I'm using it too, even if the SEXP returned by mkChar()
is not protected.
Same here:

  SET_ELEMENT(ans, 0, duplicate(x));

The SEXP returned by duplicate() is not protected.

So everybody seems to assume that SET_ELEMENT(), SET_STRING_ELT(),
SET_NAMES(), etc... can't (and will never) trigger garbage collection.
But what about defineVar()? More generally, how do I know this for the
functions/macros listed in Rdefines.h and Rinternals.h?

Thanks!
H.

> 
>>   
>>> (I don't think you need to  PROTECT elements in the symbol table)
>>>
>>>     
>>   
> 
>



[Rd] Pb with defineVar() example in the "Writing R Extensions" manual
Herve Pages