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

[Duncan Murdoch]

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.

Duncan Murdoch

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