-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
wren-cli should fflush(0) before returning with a successful exit code #56
Comments
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.) |
Is there a way to reproduce the test case on OS X which has no |
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. |
In every case I've tried the shell prints an error:
|
I stand corrected! This test did not work as I had intended. The message
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: 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: 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. |
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. |
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
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. |
Thanks for the very detailed write-up. :-)
Did you test the change to see if it errors out as you would expect it to?
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. :-) |
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:
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
will be nearly as good and avoids all the complexities with errno/sterror handling. |
@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. |
I would like to use wren-cli for writing real scripts that work reliably, not just toying around for learning the languages and trying out things.
Unfortunately, wren-cli is not reliable yet because it fails to detect write errors in certain situations. And reliable programs must catch at least input/output errors!
In order to demonstrate the problem, compare
versus
wren should also display an error message in this case, and could even surpass tclsh by also returning EXIT_FAILURE in this case, allowing a script invoking wren-cli to detect the failure and react accordingly.
The reason why wren-cli currently misses this error is because it obviously does not call fflush(0) and check its success before exiting.
The problem are the C standard I/O functions which normally work buffered. Which means a printf() might be successful, even tough it did not write anything yet. It only wrote to the internal buffer.
When main() returns, the C runtime automatically calls fflush(0) internally, which tries to write out the contents all yet-unwritten buffers. This fails, but at this time main() has already exited and cannot detect the error any more. It is implementation-defined what the C runtime does in this situation. Under Linux obviously it ignores the error and returns as if everything was fine.
But if wren called fflush() itself before exit() or returning from main(), it had a chance to catch the write error and to report it or throw an exception.
I would very much appreciate if wren-cli would do this, because then it would be possible to write scripts that catch all output errors - and not just most.
The text was updated successfully, but these errors were encountered: