From murdoch.duncan at gmail.com  Mon Jul 10 11:19:30 2017
From: murdoch.duncan at gmail.com (Duncan Murdoch)
Date: Mon, 10 Jul 2017 05:19:30 -0400
Subject: [Rd] [New Patch] Fix disk corruption when writing
In-Reply-To: <ce5bb61d-0eb4-cc7d-49ec-d43b8de14eab@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>
 <e8804ff7-473f-3b89-fa65-c1b0d79d85e3@gmail.com>
 <e1cf1c31-2e7c-72cb-453f-68df8321cc3f@insa-toulouse.fr>
 <8ca82104-19ae-a7be-d27f-9f2982b63140@gmail.com>
 <ce8dbd19-c6f1-fcf0-c9af-fccc27eb37c8@insa-toulouse.fr>
 <02239f51-3592-d3f3-5f4a-f479b35b4da9@gmail.com>
 <a5920916-a708-14d3-0e0f-66e5f35f7871@gmail.com>
 <ce5bb61d-0eb4-cc7d-49ec-d43b8de14eab@insa-toulouse.fr>
Message-ID: <a974c35e-aadb-c9b7-af70-f486e3c24dd2@gmail.com>

On 10/07/2017 4:54 AM, Serguei Sokol wrote:
> Le 08/07/2017 ? 00:54, Duncan Murdoch a ?crit :
>> I have now committed changes to R-devel (rev 72898) that seem to catch large and small errors.  They only give a warning if the error happens when the
>> connection is closed, because that can happen asynchronously
> For this asynchronous behavior, would not it be more useful to have
> the name of the file that failed at closing? If many files were open
> during a session and not closed explicitly (yes, bad practice but it
> can happen), the warning message doesn't help to understand
> which of files were corrupted, e.g.:
>  > fc=file("/dev/full", "w")
>  > write.csv("a", file=fc)
>  > q("yes")
> Warning message:
> In close.connection(getConnection(set[i])) :
>    Problem closing connection:  No space left on device
>
> Having only "set[i]" for indication is not very informative, is it?

To debug your failure to close fc, reproduce the conditions before the 
warning was issued, and call showConnections().

Duncan Murdoch

>
> Serguei.
>
>> : I didn't want to mess up some later unrelated computation that triggered garbage collection.
>>
>> I will wait a while before porting these to R-patched, because there may still be some problems to clean up.
>>
>> Duncan Murdoch
>>
>>
>>
>> On 07/07/2017 11:42 AM, Duncan Murdoch wrote:
>>> 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.
>>>>
>>>
>>
>>
>



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