From luke at stat.uiowa.edu  Wed Jan 16 19:15:34 2008
From: luke at stat.uiowa.edu (Luke Tierney)
Date: Wed, 16 Jan 2008 12:15:34 -0600 (CST)
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: <Pine.LNX.4.64.0801161215070.5979@itasca2.wildberry.org>

On Wed, 16 Jan 2008, 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.

CONS itself protects its arguments though.

luke

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


-- 
    O__  ---- Peter Dalgaard             ?ster Farimagsgade 5, Entr.B
   c/ /'_ --- Dept. of Biostatistics     PO Box 2099, 1014 Cph. K
  (*) \(*) -- University of Copenhagen   Denmark          Ph:  (+45) 35327918
~~~~~~~~~~ - (p.dalgaard at biostat.ku.dk)                  FAX: (+45) 35327907

______________________________________________
R-devel at r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel

-- 
Luke Tierney
Chair, Statistics and Actuarial Science
Ralph E. Wareham Professor of Mathematical Sciences
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


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