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

[Sokol Serguei]

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

Serguei.

>>
>> Le 05/07/2017 à 15:33, Serguei Sokol a écrit :
>>> Le 05/07/2017 à 14:46, Serguei Sokol a écrit :
>>>> Le 05/07/2017 à 13:09, Duncan Murdoch a écrit :
>>>>> On 05/07/2017 5:26 AM, January W. wrote:
>>>>>> I tried the newest patch, but it does not seem to work for me (on
>>>>>> Linux). Despite the check in Rconn_printf, the write.csv happily 
>>>>>> writes
>>>>>> to /dev/full and does not report an error. When I added a 
>>>>>> printf("%d\n",
>>>>>> res); to the Rconn_printf() definition, I see only positive values
>>>>>> returned by the vfprintf call.
>>>>>>
>>>>>
>>>>> That's likely because you aren't writing enough to actually 
>>>>> trigger a write to disk during the write.  Writes are buffered, 
>>>>> and the error doesn't happen
>>>>> until the buffer is written.
>>>> I can confirm this behavior with fvprintf(). Small and medium sized 
>>>> writings
>>>> on /dev/full don't trigger error and 1MB does.
>>>>
>>>> But if fprintf() is used, it returns a negative value from the very 
>>>> first byte written.
>>> I correct myself. In my test, fprintf() returned -1 for another 
>>> reason (connection was already closed
>>> at this moment)
>>> However, if fvprintf(...) is followed by res=fflush(con) then res is -1
>>> if we try to write on /dev/full. May be we have to use this to trigger
>>> an error message in R.
>>>
>>> Serguei.
>>>
>>>>>   The regression test I put in had this problem; I'm working on 
>>>>> MacOS and Windows, so I never got to actually try it before 
>>>>> committing.
>>>>>
>>>>> Unfortunately, it doesn't look possible to catch the final flush 
>>>>> of the buffer when the connection is closed, so small writes won't 
>>>>> trigger any error.
>>>>>
>>>>> It's also possible that whatever system you're on doesn't signal 
>>>>> an error when the write fails.
>>>>>
>>>>> Duncan Murdoch
>>>>>
>>>>>> Cheers,
>>>>>>
>>>>>> j.
>>>>>>
>>>>>>
>>>>>> On 4 July 2017 at 21:37, Duncan Murdoch <murdoch.duncan at gmail.com
>>>>>> <mailto:murdoch.duncan at gmail.com>> wrote:
>>>>>>
>>>>>>     On 04/07/2017 11:50 AM, Jean-Sébastien Bevilacqua wrote:
>>>>>>
>>>>>>         Hello,
>>>>>>         You can find here a patch to fix disk corruption.
>>>>>>         When your disk is full, the write function exit without 
>>>>>> error
>>>>>>         but the file
>>>>>>         is truncated.
>>>>>>
>>>>>> https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17243
>>>>>> <https://bugs.r-project.org/bugzilla/show_bug.cgi?id=17243>
>>>>>>
>>>>>>
>>>>>>     Thanks.  I didn't see that when it came through (or did and 
>>>>>> forgot).
>>>>>>     I'll probably move the error check to a lower level (in the
>>>>>>     Rconn_printf function), if tests show that works.
>>>>>>
>>>>>>     Duncan Murdoch
>>>>>>
>>>>>>
>>>>>>     ______________________________________________
>>>>>>     R-devel at r-project.org <mailto:R-devel at r-project.org> mailing 
>>>>>> list
>>>>>>     https://stat.ethz.ch/mailman/listinfo/r-devel
>>>>>> <https://stat.ethz.ch/mailman/listinfo/r-devel>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> -- 
>>>>>> -------- January Weiner --------------------------------------
>>>>>
>>>>> ______________________________________________
>>>>> R-devel at r-project.org mailing list
>>>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>>>
>>>>
>>>
>>
>
>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: file_close.patch
Type: text/x-patch
Size: 384 bytes
Desc: not available
URL: <https://stat.ethz.ch/pipermail/r-devel/attachments/20170707/336be5fc/attachment.bin>