From sokol at insa-toulouse.fr  Fri Jul  7 17:13:15 2017
From: sokol at insa-toulouse.fr (Serguei Sokol)
Date: Fri, 7 Jul 2017 17:13:15 +0200
Subject: [Rd] [New Patch] Fix disk corruption when writing
In-Reply-To: <8ca82104-19ae-a7be-d27f-9f2982b63140@gmail.com>
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>
 <e8804ff7-473f-3b89-fa65-c1b0d79d85e3@gmail.com>
 <e1cf1c31-2e7c-72cb-453f-68df8321cc3f@insa-toulouse.fr>
 <8ca82104-19ae-a7be-d27f-9f2982b63140@gmail.com>
Message-ID: <ce8dbd19-c6f1-fcf0-c9af-fccc27eb37c8@insa-toulouse.fr>

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

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.



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