[New Patch] Fix disk corruption when writing

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.