Skip to content
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

echo may silently fail to write to stdout #16366

Open
iffy opened this issue Dec 15, 2020 · 2 comments
Open

echo may silently fail to write to stdout #16366

iffy opened this issue Dec 15, 2020 · 2 comments

Comments

@iffy
Copy link
Contributor

iffy commented Dec 15, 2020

echo may fail to write to stdout. When it does, it causes stdout to be put into an error state (such that c_ferror(stdout) is non-zero) but does not raise any exception. Then a future, possibly far-distant attempt to stdout.write will fail with an exception. Having the error reported so far from when the failure happened is difficult to debug.

Example

I'm having trouble duplicating the setup that caused this (it involves spawning several subprocesses, some of which set stdout to nonblocking). I was seeing errno 35, resource busy.

Hopefully you can see that errors are ignored by looking at the code for echo. Lines 801, 803 and 804 are where failed writes might happen (note the discard on each line).

Nim/lib/system/io.nim

Lines 797 to 804 in d15f63a

for s in args:
when defined(windows):
writeWindows(stdout, s)
else:
discard c_fwrite(s.cstring, cast[csize_t](s.len), 1, stdout)
const linefeed = "\n"
discard c_fwrite(linefeed.cstring, linefeed.len, 1, stdout)
discard c_fflush(stdout)

Suppose the following happens:

  1. Line 801 c_fwrite(...) fails to write.
  2. It sets errno = 35
  3. It also sets the stdout error flag such that c_ferror(stdout) == 1
  4. c_ferror is not checked within echo
  5. More code runs, possibly code that causes errno to be set to something else. In my case it was errno = 2.
  6. Finally, a call to stdout.write(...) is attempted
  7. Because stdout.write checks c_ferror(stdout), it will see there's a problem and report the error using the now-invalid errno == 2.

Possible solutions

I don't know what the solution is. I can understand why it's desirable to have echo rarely raise exceptions, especially as a debugging tool. I'm just now hesitant to use echo for anything but debugging.

  1. Make echo raise an exception if c_fwrite fails
  2. Make echo write a warning to stderr if c_fwrite fails (but don't raise an exception)
  3. Add a warning to the echo docs, indicating that failures are swallowed
  4. Possibly introduce an alternate version of echo (maybeEcho?) if it's desirable to keep echo the way it is

Additional Information

The reason my echo was failing is specific to macOS, but I believe this issue applies to any POSIX system.

$ nim -v
Nim Compiler Version 1.4.0 [MacOSX: amd64]
Compiled at 2020-10-16
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: 018ae963ba83934a68d815c3c1c44c06e8ec6178
active boot switches: -d:release

@timotheecour
Copy link
Member

timotheecour commented Dec 15, 2020

The most sensible is to raise an exception, swallowing errors always creates more problems than it solves and as you mentioned leads to difficult debugging because cause and effect can be spread far apart.

can you write a PR to fix this by raising an IOError, and see what breaks?
We could also add some logic (controlled by a flag -d:nimIoFallback:path) that would write to path when stdout and stderr is in invalid state, so output doesn't get lost (making debugging even harder)

refs timotheecour#380

somewhat related

#10343

@timotheecour
Copy link
Member

fixed by #16367, feel free to re-open if you disagree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants