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

[Duncan Murdoch]

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.
>