http://d.puremagic.com/issues/show_bug.cgi?id=10344

           Summary: Exiting _Dmain should flush all FILE*s and return
                    nonzero on failure
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: andrei@erdani.com


--- Comment #0 from Andrei Alexandrescu <andrei@erdani.com> 2013-06-12 09:33:32 PDT ---
Consider:

// file test.d
import std.stdio;
void main()
{
    writeln("test");
}

This program may be run with a file that is not writable like this:

./test 1</dev/null

Although writeln() is checked, the program appears to succeed (returning 0 to
the OS) because of buffering: (a) /dev/null is not line-buffered, and (b)
flushing files upon exiting the program is unchecked.

To wit, this will indeed work correctly in all scenarios:

// file test.d
import std.stdio;
int main()
{
    writeln("test");
    return fflush(null) != 0;
}

We should add the call to fflush to main() and return nonzero IF AND ONLY IF:
(a) fflush(null) fails, and (b) the program would have otherwise returned 0.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10344


Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com


--- Comment #1 from Steven Schveighoffer <schveiguy@yahoo.com> 2013-06-12 12:09:06 PDT ---
On one hand, I disagree that stdout failing to flush on program exit should trump whatever main returns.  The application simply may not care.

On the other hand, if stdout is invalid, and any output is done, writeln could fail itself at any time.  If this is not expected, the effect is the same.

This can be opt-out also:

int main()
{
   ....
   try { fflush(NULL); } catch {}
   return 0;
}

So I'm OK with this.

I still think it's worth checking on first write whether the file descriptor is valid (this only needs to be done for File instances opened with an existing FILE * or file descriptor).  The advantage here is that the error occurs at first use, not at some time later (possibly at program end!).  If the user is simply using an invalid file descriptor, I'd rather see the exception at use, not on program exit.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10344



--- Comment #2 from Andrei Alexandrescu <andrei@erdani.com> 2013-06-12 12:15:11 PDT ---
(In reply to comment #1)
> On one hand, I disagree that stdout failing to flush on program exit should trump whatever main returns.  The application simply may not care.

I just tested this:

import std.stdio;
int main()
{
    return fflush(null);
}

Fortunately it never fails, which is quite awesome because it means it'll fail only if the application attempted to produce output but was unable to.

If the upstream app has no interest in whether the invoked app produced proper output, they can ignore the exit code.

> On the other hand, if stdout is invalid, and any output is done, writeln could fail itself at any time.  If this is not expected, the effect is the same.
> 
> This can be opt-out also:
> 
> int main()
> {
>    ....
>    try { fflush(NULL); } catch {}
>    return 0;
> }
> 
> So I'm OK with this.

fflush doesn't throw.

> I still think it's worth checking on first write whether the file descriptor is valid (this only needs to be done for File instances opened with an existing FILE * or file descriptor).  The advantage here is that the error occurs at first use, not at some time later (possibly at program end!).  If the user is simply using an invalid file descriptor, I'd rather see the exception at use, not on program exit.

That's nice, but I'm not sure whether we should do it.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10344



--- Comment #3 from Steven Schveighoffer <schveiguy@yahoo.com> 2013-06-13 07:25:43 PDT ---
(In reply to comment #2)
> (In reply to comment #1)
> > On one hand, I disagree that stdout failing to flush on program exit should trump whatever main returns.  The application simply may not care.
> 
> I just tested this:
> 
> import std.stdio;
> int main()
> {
>     return fflush(null);
> }
> 
> Fortunately it never fails, which is quite awesome because it means it'll fail only if the application attempted to produce output but was unable to.

Right, this is definitely ideal.  One should not throw an error because stdout is broken if you never use stdout.

But that was not my point.  My point is that what you write in stdout may not be critical.  It may be a side effect, like let's say a log or informational message.  In that case, you don't care if it fails, you are more interested in what the program decides the error code should be.

For instance, imagine a program that takes as input some database commands, and prints out how many rows were updated, but returns 1 if the database commands failed, and 0 if it succeeded.  I don't care as an upstream script function whether "5 rows updated" successfully printed, I only care if the database was successfully updated.

> > This can be opt-out also:
> > 
> > int main()
> > {
> >    ....
> >    try { fflush(NULL); } catch {}
> >    return 0;
> > }
> > 
> > So I'm OK with this.
> 
> fflush doesn't throw.

oops, I meant stdout.flush(); stderr.flush();

> > I still think it's worth checking on first write whether the file descriptor is valid (this only needs to be done for File instances opened with an existing FILE * or file descriptor).  The advantage here is that the error occurs at first use, not at some time later (possibly at program end!).  If the user is simply using an invalid file descriptor, I'd rather see the exception at use, not on program exit.
> 
> That's nice, but I'm not sure whether we should do it.

It's simply an optimization.  There is little overhead required (a simple bool indicating whether it should check the fd on first write).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10344


Koroskin Denis <2korden@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |2korden@gmail.com


--- Comment #4 from Koroskin Denis <2korden@gmail.com> 2013-06-14 13:43:26 PDT ---
Will that affect int main() { .. } functions as well?

Consider the following example:

int main()
{
   Result result = doLogic(); // does some logic, uses writeln() to indicate
progress
   return mapResultToReturnCode(result);
}

Let's say it's a video encoding program, and is a part of a longer toolchain (some other program will do something else based on return code).

Will writeln()/flush() hijack main return code even though the programmer
EXPLICITLY returned something else?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10344


Peter Alexander <peter.alexander.au@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |peter.alexander.au@gmail.co
                   |                            |m


--- Comment #5 from Peter Alexander <peter.alexander.au@gmail.com> 2013-06-14 14:47:41 PDT ---
(In reply to comment #4)
> Will writeln()/flush() hijack main return code even though the programmer
> EXPLICITLY returned something else?

At the end of the original post, Andrei wrote:

"We should add the call to fflush to main() and return nonzero IF AND ONLY IF:
(a) fflush(null) fails, and (b) the program would have otherwise returned 0."

So, it will only hijack the return code if the return code was 0.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10344



--- Comment #6 from Steven Schveighoffer <schveiguy@yahoo.com> 2013-06-14 15:48:41 PDT ---
(In reply to comment #5)

> So, it will only hijack the return code if the return code was 0.

What is OK about overriding the return code of 0, but not an error return code?
 If stdout output is not critical, or even viewed, then there is no significant
error.  Hijacking 0 when the program actually succeeded is not preferred.

I will say that *relying* on this behavior is a programming error -- it means the program is relying on an implementation detail (buffer size) to NOT throw an exception during normal execution.  So from that point of view, it kind of makes sense that one would want to see an error rather than not.

But the proposed solution doesn't seem to be any better.  It may end up simply breaking something that was working fine (for that particular usage) before the change, for no good reason.   In fact, any change we make can be viewed this way (throwing on first write, for instance).

I'm actually feeling more like we just should do nothing, because the use case is mostly invalid (starting out with an invalid file descriptor), and the error will be more annoying than useful.  The corner cases are so rare and vague that I don't see this helping anyone.

It should be noted that this whole discussion came about because a developer naively assumed that the code posted in the description was attempting to write to the file descriptor, ignoring the failure, and continuing.  When actuality, the test is wrong.  They just didn't understand that C intelligently decides not to flush after newlines when the output is not a console.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10344



--- Comment #7 from Andrei Alexandrescu <andrei@erdani.com> 2013-06-14 18:45:20 PDT ---
(In reply to comment #6)
> (In reply to comment #5)
> 
> > So, it will only hijack the return code if the return code was 0.
> 
> What is OK about overriding the return code of 0, but not an error return code?

If the program was to return an error code, it meant it has failed for whatever reason. The fact that among other failures it also failed to flush is of no consequence.

>  If stdout output is not critical, or even viewed, then there is no significant
> error.

We're not to decide what is critical to the program and what's not. We are to report an unexpected error.

> Hijacking 0 when the program actually succeeded is not preferred.

By whom? Why? What's the purpose of saying this? This is just a statement without any justification.

> I will say that *relying* on this behavior is a programming error -- it means the program is relying on an implementation detail (buffer size) to NOT throw an exception during normal execution.  So from that point of view, it kind of makes sense that one would want to see an error rather than not.
> 
> But the proposed solution doesn't seem to be any better.  It may end up simply breaking something that was working fine (for that particular usage) before the change, for no good reason.   In fact, any change we make can be viewed this way (throwing on first write, for instance).
> 
> I'm actually feeling more like we just should do nothing, because the use case is mostly invalid (starting out with an invalid file descriptor), and the error will be more annoying than useful.  The corner cases are so rare and vague that I don't see this helping anyone.
> 
> It should be noted that this whole discussion came about because a developer naively assumed that the code posted in the description was attempting to write to the file descriptor, ignoring the failure, and continuing.  When actuality, the test is wrong.  They just didn't understand that C intelligently decides not to flush after newlines when the output is not a console.

I think you are wrong in several obvious ways, plus a few subtler ones.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10344



--- Comment #8 from Steven Schveighoffer <schveiguy@yahoo.com> 2013-06-17 06:21:27 PDT ---
(In reply to comment #7)
> (In reply to comment #6)
> >  If stdout output is not critical, or even viewed, then there is no significant
> > error.
> 
> We're not to decide what is critical to the program and what's not. We are to report an unexpected error.

But you are hijacking the return value, effectively declaring that stdout success is a critical error.

> > Hijacking 0 when the program actually succeeded is not preferred.
> 
> By whom? Why? What's the purpose of saying this? This is just a statement without any justification.

I'll bring it up again, maybe you missed it:

Let's say there is a program that executes some database commands, then outputs how many rows affected.  It returns 0 on success to modify the database, 1 on failure.  The output of how many rows affected is likely less than 100 chars. So it likely would not flush before program termination.

Now, consider that the author of this program has no idea about the "flush-at-end-of-main" feature (an extremely likely scenario).  All his testing does not involve creating full disks, or invalid descriptors for stdout.  The man page and help screen likely says that it returns 0 on successful write to the DB, non-zero on failure.  It does not mention that if it fails to output "X rows affected," an error is returned.

Someone using this program messes up and passes an invalid descriptor (for whatever reason).  What happens is that any program using this application will see a failure, yet the database commands had succeeded.  What is the utility in that?  It's a completely unexpected occurrence, from both sides, and it even violates the documentation.  For all intents and purposes, you have introduced a bug into every program that doesn't expect this behavior.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10344



--- Comment #9 from Andrei Alexandrescu <andrei@erdani.com> 2013-06-17 08:12:56 PDT ---
(In reply to comment #8)
> Let's say there is a program that executes some database commands, then outputs how many rows affected.  It returns 0 on success to modify the database, 1 on failure.  The output of how many rows affected is likely less than 100 chars. So it likely would not flush before program termination.
> 
> Now, consider that the author of this program has no idea about the "flush-at-end-of-main" feature (an extremely likely scenario).  All his testing does not involve creating full disks, or invalid descriptors for stdout.  The man page and help screen likely says that it returns 0 on successful write to the DB, non-zero on failure.  It does not mention that if it fails to output "X rows affected," an error is returned.
> 
> Someone using this program messes up and passes an invalid descriptor (for whatever reason).  What happens is that any program using this application will see a failure, yet the database commands had succeeded.  What is the utility in that?  It's a completely unexpected occurrence, from both sides, and it even violates the documentation.  For all intents and purposes, you have introduced a bug into every program that doesn't expect this behavior.

But this is an argument against our current approach to stdout (we check and throw on all errors). The error in the program you mentioned may very well intervene in writeln under one or more of the following conditions:

* sufficient characters output
* line buffered or unbuffered stdout

In such cases, succeeding to produce stdout is just as unimportant to the program, but the program will fail with nonzero error code. Are you saying we should change that?

At the highest level, it is well understood there examples can be given that show the stdout output was of small consequence to the program. The point is we cannot decide on behalf of the program that it's okay to have truncated output.

In D all flushes of stdout are checked and exceptions are thrown if they fail. The last flush is not special in any way and must behave the same.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------