From simon.urbanek at r-project.org  Sat Nov 10 22:27:38 2007
From: simon.urbanek at r-project.org (Simon Urbanek)
Date: Sat, 10 Nov 2007 16:27:38 -0500
Subject: [Rd] C/C++ 'assert' should not be used in R packages
In-Reply-To: <4735F302.501@stats.uwo.ca>
References: <Pine.LNX.4.64.0711071638510.7944@gannet.stats.ox.ac.uk>
	<47359829.8090909@stats.uwo.ca>
	<4735F1C0.8050601@wald.ucdavis.edu> <4735F302.501@stats.uwo.ca>
Message-ID: <AF618CB3-93F7-439C-AEBC-F84E3904E0CB@r-project.org>


On Nov 10, 2007, at 1:05 PM, Duncan Murdoch wrote:

> On 10/11/2007 1:00 PM, Duncan Temple Lang wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>>
>>
>> Duncan Murdoch wrote:
>>> Prof Brian Ripley wrote:
>>>> Please don't use 'assert' in R packages.  If called, this means  
>>>> that an
>>>> error in your code aborts the whole R process, including your  
>>>> user's work.
>>>> I see several R packages doing this, and one of them called  
>>>> 'assert' on me
>>>> earlier in the week.
>>>>
>>> I partly disagree about this.  If assert() is triggered, it clearly
>>> indicates a bug in the package.  If it just generated an R error,  
>>> most
>>> users would ignore it, and not report it to the package maintainer.
>>>
>>> It may well be that when an assertion fails, none of the subsequent
>>> calculations are reliable, in which case returning control to the  
>>> user
>>> could result in data corruption.  That's worse than losing a  
>>> session,
>>> because at least when you lose a session, you know it.
>>>
>>> Could we write our own implementation of assert() that displays an R
>>> error and unloads the package?  I think I could do something like  
>>> that
>>> in Windows by calling FreeLibrary to unload the DLL, but I'd  
>>> prefer a
>>> cross-platform solution.
>>
>> I am not sure why you think we need to discard the DLL.
>
> The package author has asserted that it is no longer safe to run.   
> They are shutting down R, and Brian finds that too extreme.
>

Me too. A package should never intentionally close the R session. If  
it does, it is a very severe design bug in the package (IMHO).  
Anything you use assert for can be gracefully handled (as opposed to  
let's say a segfault), so there is no excuse for killing the whole  
process at all (deliberately wiping all user's work is rude to put it  
mildly). Even if it might be unsafe to run anything, I'd still want  
to attempt to save my data.

Cheers,
Simon


>> What if we want to access variables and routines merely to find out
>> the resulting state after the assert().
>> And it is not clear whether you mean any "subsequent calculations"
>> or any "subsequent calculations using code within this DLL"
>> might be unreliable.
>
> How could I know why a package author decided a shutdown is  
> needed?  It
> depends on the author.
>
>>
>> But regardless of that and the assumption that users will ignore this
>> error, why would you want to call FreeLibrary? Since R has induced  
>> the
>> DLL to be loaded either directly or indirectly, R holds a handle  
>> to the
>> DLL. If you don't use R's cross-platform facilities for releasing the
>> DLL (either at the R or C-level), you will corrupt the session,
>> specifically in the list of assumed-live DLLs.
>
> As I said, I'd prefer a cross-platform solution.
>
> Duncan Murdoch
>>
>>  D.
>>
>>
>>>> We provide 'error': please do use it to return control to the  
>>>> user when
>>>> your code misbehaves.
>>>>
>>>> Similarly 'exit' and 'abort' should never be used in R packages.
>>>>
>>>>
>>>> Sometimes it is not under your control: I sometimes see an rgl  
>>>> failure at
>>>>
>>>> R: indirect_vertex_array.c:659: emit_DrawArrays_old: Assertion
>>>> `elements_per_request >= count' failed.
>>>>
>>>> that is coming from the Mesa GL libraries.
>>>>
>>> I'd say that's a bug, either in Mesa GL or in rgl.  If you can  
>>> make it
>>> reproducible, I'll try to track it down.
>>>
>>> Duncan Murdoch
>>>
>>> ______________________________________________
>>> R-devel at r-project.org mailing list
>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG v1.4.7 (Darwin)
>> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
>>
>> iD8DBQFHNfHA9p/Jzwa2QP4RAo2BAJkB5QaFLOW5fcR6QE9JSXpIWAcEUwCggMYK
>> 2WBnl5jDgTUayU5wk8hnQj4=
>> =91by
>> -----END PGP SIGNATURE-----
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>



[Rd] C/C++ 'assert' should not be used in R packages
Simon Urbanek