[Rd] [New Patch] Fix disk corruption when writing

[Duncan Murdoch]

I have now committed changes to R-devel (rev 72898) that seem to catch 
large and small errors.  They only give a warning if the error happens 
when the connection is closed, because that can happen asynchronously: 
I didn't want to mess up some later unrelated computation that triggered 
garbage collection.

I will wait a while before porting these to R-patched, because there may 
still be some problems to clean up.

Duncan Murdoch



On 07/07/2017 11:42 AM, Duncan Murdoch wrote:
> On 07/07/2017 11:13 AM, Serguei Sokol wrote:
>> Le 07/07/2017 à 16:52, Duncan Murdoch a écrit :
>>> On 07/07/2017 9:54 AM, Serguei Sokol wrote:
>>>> Le 07/07/2017 à 01:09, Duncan Murdoch a écrit :
>>>>> On 06/07/2017 6:44 PM, Sokol Serguei wrote:
>>>>>> Duncan Murdoch has written at  Thu, 6 Jul 2017 13:58:10 -0400
>>>>>>> On 06/07/2017 5:21 AM, Serguei Sokol wrote:
>>>>>>>> I propose the following patch against the current
>>>>>>>> R-devel/src/main/connection.c (cf. attached file).
>>>>>>>> It gives (on my linux box):
>>>>>>>>  > fc=file("/dev/full", "w")
>>>>>>>>  > write.csv("a", file=fc)
>>>>>>>> Error in writeLines(paste(col.names, collapse = sep), file, sep = eol) :
>>>>>>>>    system call failure on writing
>>>>>>>>  > close(fc)
>>>>>>>>
>>>>>>>> Serguei.
>>>>>>>
>>>>>>> I suspect that flushing on every write will slow things down too much.
>>>>>> That's quite plausible.
>>>>>>
>>>>>>>
>>>>>>> I think a better approach is to catch the error in the Rconn_printf
>>>>>>> calls (as R-devel currently does), and also catch errors on
>>>>>>> con->close(con).  This one requires more changes to the source, so it
>>>>>>> may be a day or two before I commit.
>>>>>> I have testes it on file_close() and it works (cf. attached patch):
>>>>>>> fc=file("/dev/full", "w")
>>>>>>> write.csv("a", file=fc)
>>>>>>> close(fc)
>>>>>> Error in close.connection(fc) : closing file failed
>>>>>>
>>>>>>>
>>>>>>> One thing I have to look into:  is anyone making use of the fact that
>>>>>>> the R-level close.connection() function can return -1 to signal an
>>>>>>> error?  Base R ignores that, which is why we need to do something, but
>>>>>>> if packages are using it, things need to be changed carefully.  I
>>>>>>> can't just change it to raise an error instead.
>>>>>> As you can see in the patch, no need to change close.connection() function
>>>>>> but we have to add a test of con->status to all *_close() functions
>>>>>> (gzfile_close() and co.)
>>>>>
>>>>> You missed my point.  Currently the R close() function may return -1 to signal that there was an error closing.  We can't change that to an error if packages
>>>>> are using it.
>>>> May be I missed it but finally, me too, I was saying that we don't have to do so.
>>>> Anyhow, the situation of writing to full disk cannot be passed in silence.
>>>> IMHO, trigger an error would be the most appropriate in this situation but if for legacy
>>>> or any other reason we cannot do so, let whistle a warning, at least.
>>>> Here few tests with a new small patch:
>>>>  > fc=file("/dev/full", "w"); write.csv("a", file=fc); (res=close(fc))
>>>> [1] -1
>>>> Warning message:
>>>> In close.connection(fc) :
>>>>    closing '/dev/full' failed: No space left on device
>>>>  > fc=gzfile("/dev/full", "w"); write.csv("a", file=fc); (res=close(fc))
>>>> NULL
>>>> Warning message:
>>>> In close.connection(fc) :
>>>>    closing '/dev/full' failed: No space left on device
>>>>  > fc=xzfile("/dev/full", "w"); write.csv("a", file=fc); (res=close(fc))
>>>> NULL
>>>> Warning message:
>>>> In close.connection(fc) :
>>>>    closing '/dev/full' failed: No space left on device
>>>>  > fc=bzfile("/dev/full", "w"); write.csv("a", file=fc); (res=close(fc))
>>>> NULL
>>>> Warning message:
>>>> In close.connection(fc) :
>>>>    closing '/dev/full' failed: No space left on device
>>>>
>>>> Note that if we test only status < 0 (without errno) then there are too many warnings
>>>> on seemingly "innocent" file closings.
>>>
>>> Could you give an example of how to get status < 0 on a valid closing?
>> If you remove "&& errno" and leave only "if (status < 0)" in the previous patch
>> then during make I have many warnings, e.g. :
>> Warning messages:
>> 1: In close.connection(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/compiler/Meta/nsInfo.rds' failed: Success
>> 2: In close.connection(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/compiler/Meta/package.rds' failed: Success
>> 3: In close(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/compiler/R/compiler.rdx' failed: Success
>> 4: In close.connection(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/tools/Meta/nsInfo.rds' failed: Success
>> 5: In close.connection(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/tools/Meta/package.rds' failed: Success
>> 6: In close(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/tools/R/tools.rdx' failed: Success
>> 7: In close(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/tools/R/sysdata.rdx' failed: Success
>> 8: In close.connection(con) :
>>    closing '../../library/parallel/Meta/Rd.rds' failed: Success
>> 9: In close.connection(con) :
>>    closing '../../library/parallel/help/aliases.rds' failed: Success
>> 10: In close.connection(file) :
>>    closing '../../library/parallel/DESCRIPTION' failed: Success
>>
>
> You are probably seeing cases where status is never set:  then status is
> NA_INTEGER, which (in C) is negative.
>
> Duncan Murdoch
>
>
>> Note "Succes" as the reason of "failure".
>>
>> And if I use thus compiled R, at startup I get:
>>
>> Warning message:
>> In close(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/base/R/base.rdx' failed: Success
>>
>> R Under development (unstable) (2017-06-01 r72753) -- "Unsuffered Consequences"
>> Copyright (C) 2017 The R Foundation for Statistical Computing
>> Platform: x86_64-pc-linux-gnu (64-bit)
>>
>> R is free software and comes with ABSOLUTELY NO WARRANTY.
>> You are welcome to redistribute it under certain conditions.
>> Type 'license()' or 'licence()' for distribution details.
>>
>> R is a collaborative project with many contributors.
>> Type 'contributors()' for more information and
>> 'citation()' on how to cite R or R packages in publications.
>>
>> Type 'demo()' for some demos, 'help()' for on-line help, or
>> 'help.start()' for an HTML browser interface to help.
>> Type 'q()' to quit R.
>>
>> Warning messages:
>> 1: In close.connection(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/methods/Meta/package.rds' failed: Success
>> 2: In close.connection(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/methods/Meta/features.rds' failed: Success
>> 3: In close.connection(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/methods/Meta/nsInfo.rds' failed: Success
>> 4: In close.connection(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/methods/Meta/package.rds' failed: Success
>> 5: In close(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/methods/R/methods.rdx' failed: Success
>> [Previously saved workspace restored]
>>
>> During startup - There were 27 warnings (use warnings() to see them)
>>
>> All these closings seem valid to me but obviously the warnings indicate that status was < 0.
>>
>> Finaly, if I open and close a valid file in this session, I don't get a warning for _this_ file:
>>  > fc=file("/tmp/aha", "w")
>> Warning messages:
>> 1: In close.connection(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/compiler/Meta/nsInfo.rds' failed: Success
>> 2: In close.connection(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/compiler/Meta/package.rds' failed: Success
>> 3: In close(con) :
>>    closing '/home/sokol/dev/R/R-devel/library/compiler/R/compiler.rdx' failed: Success
>>  > write.csv("a", file=fc)
>>  > close(fc) # no warning
>>
>>
>> Serguei.
>>
>