wren-cli should fflush(0) before returning with a successful exit code about wren-cli HOT 13 OPEN

I used "wren0" in the example command line of my initial posting for this issue.

This was mistake, "wren-cli" was meant.

(I have locally renamed wren-cli into wren0, because 0 is wren's current major version number. This allows me to install multiple wren versions side by side if wren 1.x comes out later. New scripts will then invoke the newer version as wren1, while the old scripts still can use wren0 and both will be installed.)

from wren-cli.

Is there a way to reproduce the test case on OS X which has no /dev/full?

from wren-cli.

Is there a way to reproduce the test case on OS X which has no /dev/full?

Sure. Just try to write to any filesystem which is already completely full.

For instance, you could mount an USB thumb drive, fill it with data up to the last free byte, and then try to redirect output to a new file on it which will obviously fail.

However, writing to /dev/full is not the only way to check whether a program checks for I/O errors.

You could also try to append output to a file for which you have no write permission. This should also result in an error message, although it might be a different one.

If a program continues after such an attempt as if nothing ever happened, then you know this program does not check for I/O errors properly.

from wren-cli.

In every case I've tried the shell prints an error:

% ./bin/wren_cli test.wren > x             
<W> fish: An error occurred while redirecting file 'x'
open: Permission denied

from wren-cli.

I stand corrected!

This test did not work as I had intended.

The message

fish: An error occurred while redirecting file 'x'
open: Permission denied

shows that not the wren test program produced this error message but rather the "fish" shell which you obviously had been using (perhaps via "mc"?) for the test.

In other words, my advice was bad advice: Permissions for redirections are checked too soon.

The shell caught the error before the test program could even be run, and therefore tells nothing about what wren would have done when writing to the file.

Sorry about that - I did not think this through thoughly enough.

However, the method with the full USB thumb drive (or any other filesystem which is completely full) should still work.

Hmmm.

Or maybe not: If the filesystem was completely full, the shell would also catch the error when attempting to create the new output file. And not the test program.

Which means, there should be enough space to create an empty new file on the filesystem, but not enough to write even a single byte to it.

This may be tricky.

Therefore a better idea: First create a file filling up all available space, and then try to append data to it:
$ cat /dev/zero > /mnt/usb_stick/filler

will create a file "filler" on the mounted thumb drive which fills up all the space. This command will eventually die with an error message, but all space on the fileystem will be filled.

Then you can try the wren test program:
$ /bin/wren_cli test.wren >> /mnt/usb_stick/filler

Note the >> redirection for appending rather than > for creating a new file.

The >> redirection should not give any troubles for the shell, because the file already exists and nothing has been written yet.

But as soon as the wren program tries to write the first byte, an "out of space"-kind of error message should be the result. Because there is no space left to actually append any new data to the file.

The above exampes assume your thumb drive has been mounted as /mnt/usb_stick. Change that path to match your mount actual point.

from wren-cli.

Bash had the same behavior. If you find a simpler reproducible test case that doesn't require filling a disk, let me know. Otherwise I'll probably focus on some of the other things that are more obvious.

I wouldn't think this would be super hard to patch though and test yourself.

from wren-cli.

Does just calling Stdout.flush() from your script with the patch in PR #68 produce the results you want?

from wren-cli.

Does just calling Stdout.flush() from your script with the patch in PR #68 produce the results you want?

It is at least one of the things to do.

C actually has the same problem - just calling printf() and checking its return code does not guarantee all write errors can be detected because of C's output buffering mechanism which can delay the actual output.

The problem here is that no output actually happens at the time when printf() is called - the data are just written to the output buffer, but the buffer itself is not yet been written out. Thus no output error yet, even if the disk is already full.

If this happens at the end of the program, the output buffer will eventually be flushed out by the C runtime which cleans up the resources used by the C program.

This flushing operation will then fail, but there is no-one left to handle the error, because this error is only detected after main() has already returned (or exit() has already been called).

Theoretically, the C runtime could convert this output error to a failure return code and/or display an error message. The C standard does not require this, however, and therefore such action would be implementation-specific behavior. No C runtime implementation I have encountered so far does do this. Instead, they all just ignore the error. No error message is displayed and the process return status is not changed to "not successfully".

As a consequence, in order to write a C program which catches all output errors, one must do a

if (fflush(0)) goto error;

before leaving the program.

The fflush(0) will force flushing all unwritten output buffers (not just but including stdout), forcing this to be done before the C runtime cleanup code is run which would not know how to properly handle such a situation.

The if() ensures that any output errors encounted during execution of fflush(0) will be caught, and the "goto error" jumps to some error handling code which outputs some sort of "write error" message, typically by displaying strerror(errno) in some way.

Therefore, calling Stdout.flush() an the end of wren cli's main program would definitely be a good idea and eliminate the largest problem.

However, calling the C function fflush(0) somehow would be even better, because this flushes all open output streams (and not just Stdout) which might have the same problem.

And in all cases, it is important that the return status of fflush() is checked in order to detect an output error! Otherwise calling fflush() has no point, because the C runtime will eventually call it automatically and the only reason to call it explicitly is to learn about write errors caused by it.

from wren-cli.

Thanks for the very detailed write-up. :-)

calling Stdout.flush() an the end of wren cli's main program would definitely be a good idea and eliminate the largest problem.

Did you test the change to see if it errors out as you would expect it to?

However, calling the C function fflush(0) somehow would be even better,

I wasn't saying we shouldn't do that also, just trying to make some progress here since these are filed as two separate issues. :-)

from wren-cli.

Are there standard errors codes for situations like this?

from wren-cli.

Are there standard errors codes for situations like this?

Unfortunately no. The C standard just states that fflush() returns non-zero in case of any error, but does not state that errno must be set to some particular error code (or at all).

POSIX on the other hand guarantees indeed a couple of error codes to be set via errno in case fflush would fail, which includes ENOSPC in case that the output device is already full. However, there are many more possible errno value for fflush() and ENOSPC is just one of them.

Therefore I would suggest the following which will do the right thing on POSIX as well as non-POSIX platforms:

1. set errno=0 before calling fflush
2. call fflush(0) and check its return code
3. if the result is not zero, call sterror(errno) to get the appropriate error message to display but only if errno is nonzero
4. If the result is not zero but errno is still zero, display a "write error" message instead yourself

The rationale behind this is as follows:

If fflush() does indeed set errno, use the value of errno in case of an error in order to produce the most appropriate error message via strerror()

If fflush() does not set errno, it will still be zero because it was set to zero at step 1. In this case we do not know what exactly happened if fflush() returns non-zero, only that something went wrong. However, the only likely reason why fflush() could possibly fail is a write error, and so we report one.

This would be the super-deluxe method of checking for fflush() errors, but a shorter version

int main() {
   ...
   int rc= EXIT_SUCCESS;
   ...
   if (fflush(0)) { (void)fputs("Write error!\n", stderr); rc= EXIT_FAILURE; }
   ...
   return rc;
}

will be nearly as good and avoids all the complexities with errno/sterror handling.

from wren-cli.

@guenther-brunthaler #77 Thoughts?

from wren-cli.

@guenther-brunthaler #77 Thoughts?

This patch has the potential to lift Wren from a pretty scripting language to a reliable scripting language which is well-suited for mission-critical work.

Mission-critical applications generally cannot afford to ignore I/O errors unless this is intended.

Ignoring the return code of a script intentionally is always possible, but unless the script reports each and every I/O error reliably it would not possibly to rely on the output of the script at all because one can never know whether the generated output is complete or not.

Therefore I'd like that patch very much to be integrated as soon as possible!

It would allow me to use Wren for scripts I would not have dared to use it before because I needed those scripts to be highly reliable.

from wren-cli.