R devel - Jan 2011 - [Rcpp-devel] Loading a package using Rcpp Modules results in memory corruption

On Tue, Jan 11, 2011 at 2:41 PM, Romain Francois
<romain@r-enthusiasts.com>wrote:
> Le 11/01/11 19:57, Romain Francois a écrit :
>
>  Le 11/01/11 19:46, Douglas Bates a écrit :
>>
>>> On Tue, Jan 11, 2011 at 12:27 PM, Dominick
>>> Samperi<djsamperi@gmail.com> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Jan 11, 2011 at 1:20 PM, Romain
>>>> Francois<romain@r-enthusiasts.com>
>>>> wrote:
>>>>
>>>>>
>>>>> Le 11/01/11 19:12, Davor Cubranic a écrit :
>>>>>
>>>>>>
>>>>>> I think an important point from Doug has been lost in
the subsequent
>>>>>> 20-odd messages of flamebombing:
>>>>>>
>>>>>>  I do not see this as compatible with the C++ Design
principle we use
>>>>>>>> whereby
>>>>>>>> protection / unprotection occurs relative to
the end of scope -- and
>>>>>>>> not
>>>>>>>> after every single assignment or allocation.
>>>>>>>>
>>>>>>>> In other words, gctorture() may well be a fine
test for the C API
>>>>>>>> and
>>>>>>>> its
>>>>>>>> PROTECT and UNPROTECT at every step but
possibly not quite as
>>>>>>>> much for
>>>>>>>> Rcpp.
>>>>>>>>
>>>>>>>
>>>>>>> I don't think so. In the situation that
Dominick is describing the C
>>>>>>> API is being used (calls to Rf_install, Rf_lang1,
Rf_eval, ...) and
>>>>>>> you have to play by the rules of the C API.
Essentially every time
>>>>>>> that you call a function in the C API that can
cause a memory
>>>>>>> allocation you are open yourself to the possibility
of having the
>>>>>>> garbage collector run and getting unprotected SEXPs
blown away. And,
>>>>>>> naturally, Rf_eval can cause memory allocation.
>>>>>>>
>>>>>>> I understood Dominick to be saying that in the code
related to
>>>>>>> Modules
>>>>>>> and the evaluator and all that which we have been
trying to debug
>>>>>>> there are such cases of the use of the C API that
are unsafe.
>>>>>>>
>>>>>>
>>>>>> Can anyone comment whether this is correct?
>>>>>>
>>>>>> Davor
>>>>>>
>>>>>
>>>>> Yep. Doug's analysis is right. Rcpp is implemented with
the C R API,
>>>>> and
>>>>> apparently there were a few places where we were not
careful enough.
>>>>> Most
>>>>> notably in calls to Rf_lcons and Rf_cons. This has been
partially
>>>>> dealt with
>>>>> today.
>>>>>
>>>>
>>>> Just for the record, Doug is summarizing my analysis, based on
several
>>>> examples that I posted to this thread,
>>>> and that I am pleased to see have inspired some remedial
action.
>>>>
>>>
>>> Sorry for not responding earlier. I'm in the middle of teaching
a
>>> short course.
>>>
>>> Dominick is correct that it was his analysis that picked up the
>>> failures to PROTECT/UNPROTECT in cases that were causing at least
some
>>> of the problems with loading Modules. Credit where credit is due.
>>>
>>> As Romain has indicated, some of the problems have been fixed but
>>> apparently problems still remain. Debugging memory protection
>>> problems is often very difficult.
>>>
>>
>> It is. Not sure what is my next step here. Remaining problems seem not
>> directly related to Rcpp, but rather associated with lazy loading. See:
>>
>
> This now by not be related to Rcpp. See the thread on R-devel:
> http://comments.gmane.org/gmane.comp.lang.r.devel/26504
>
> Please whoever is interested in fixing this, send your remarks and patches
> to the R-devel mailing list.
>
This looks like the same problem that appeared in Rcpp, an unprotected SEXP
in
<R>/src/main/envir.c, in the function do_as_environment(), case VECSXP of
the
switch. Here is modified code that seems to fix the problem, at least under
Linux:

    case VECSXP: {
    Rprintf("VECSXP as.environment\n");
    /* implement as.environment.list() {isObject(.) is false for a list} */
    SEXP sp = PROTECT(lang4(install("list2env"), arg,
                /*envir = */R_NilValue, /* parent = */R_EmptyEnv));
    SEXP res = eval(sp, rho);
    UNPROTECT(1);
    return res;
    }

Dominick

> Romain
>
>
> --
> Romain Francois
> Professional R Enthusiast
> +33(0) 6 28 91 30 30
> http://romainfrancois.blog.free.fr
> |- http://bit.ly/fT2rZM : highlight 0.2-5
> |- http://bit.ly/gpCSpH : Evolution of Rcpp code size
> `- http://bit.ly/hovakS : RcppGSL initial release
>
>
> _______________________________________________
> Rcpp-devel mailing list
> Rcpp-devel@lists.r-forge.r-project.org
> https://lists.r-forge.r-project.org/cgi-bin/mailman/listinfo/rcpp-devel
>
	[[alternative HTML version deleted]]