From murdoch.duncan at gmail.com  Fri Jul  7 01:09:30 2017
From: murdoch.duncan at gmail.com (Duncan Murdoch)
Date: Thu, 6 Jul 2017 19:09:30 -0400
Subject: [Rd] [New Patch] Fix disk corruption when writing
In-Reply-To: <ed2899b8-f945-1dad-348a-cee0f8614ba7@insa-toulouse.fr>
References: <CAPXDfF=770SgtaLoMeaF7XGUfj3wFSU6hUn6ZFrds0VnwG_+zA@mail.gmail.com>
 <ba1bb252-8524-ce6d-b0d7-0d66a09789d2@gmail.com>
 <CA+A1kV5xB3yfD2SnKenrnrfgV6wDObQW8vFMxUVQd2kxTc0BjQ@mail.gmail.com>
 <1093380f-a632-ce28-af21-a87e1a40a8db@gmail.com>
 <8ede4416-7745-e66c-7d2f-53c04f1a9067@insa-toulouse.fr>
 <fe2b1e03-2f57-820e-6226-217be4184907@insa-toulouse.fr>
 <0783f593-8624-6f3f-7c54-eb3ab2938a48@insa-toulouse.fr>
 <42077a0f-0e88-8400-8310-afaaa5ef6f9e@gmail.com>
 <ed2899b8-f945-1dad-348a-cee0f8614ba7@insa-toulouse.fr>
Message-ID: <e8804ff7-473f-3b89-fa65-c1b0d79d85e3@gmail.com>

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



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