Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Issue 10344 Exiting _Dmain should flush all FILE*s and return nonzero on failure #525

Closed
wants to merge 32 commits into from

Conversation

lionello
Copy link
Contributor

Flushing all open streams on exit, throwing an Error with the errno string in case of failure.

TODO: test cases. But must get test suite to work first.

{
auto s = strerror(.errno);
throw new Error(s[0..strlen(s)].idup);
}
gc_term();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we do this instead of the above:

extern (C) int main(int argc, char **argv)
{
    auto result = _d_run_main(argc, argv, &_Dmain);
    if (fflush(stdout) != 0)
    {
        fprintf(stderr, "Failed to flush stdout, error code: %d\n", errno); // unchecked
        result = 1;
    }
    fflush(stderr); // unchecked
    return result;
}

The justification goes as follows:

  1. If we flush all buffers, we'll try to flush whatever stray files the user has forgotten opened. If the user didn't care to close them, we can't assume they are of consequence to the program.
  2. stdout, however, is the runtime's responsibility because we're assume to take care of properly closing it.
  3. Not being able to flush stderr should not change the exit code because most programs do not depend on a proper stderr.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a misunderstanding here. When the C runtime gets control back from main(), it calls exit(status) which flushes all open stdio buffers. Success or failure of that does not affect the exit status.

D does not need to flush and then ignore the results of the flush - that already happens. See 7.19.3 in the C99 Standard: "If the main
function returns to its original caller, or if the exit function is called, all open files are closed (hence all output streams are flushed) before program termination. Other paths to program termination, such as calling the abort function, need not close all files
properly."

The issue we have is not that the files are not flushed - they are. The issue is what to do if the flushing failed. In C stdio, failure of the flush upon program exit is ignored.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed the flush of stderr is redundant, but other than that the code is true to form.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So let me reiterate the code I propose:

extern (C) int main(int argc, char **argv)
{
    auto result = _d_run_main(argc, argv, &_Dmain);
    if (fflush(stdout) != 0)
    {
        fprintf(stderr, "Failed to flush stdout, error code: %d\n", errno); // unchecked
        result = 1;
    }
    return result;
}

Is that at the appropriate time (i.e. all flushes etc will happen after main as far as I understand)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is at the right time, but I would amend the setting of result to be:

if (result == 0) result = 1;

@lionello
Copy link
Contributor Author

Indeed, ignoring failures for stderr seems to be the behavior of other tools I've tried (cat asdf 2</dev/null and the like), but I have a slight preference for "throw" since it can be debugged and it'll encourage the author to add proper handling to the code. And even though the exception can't be caught from D main(), I can imagine there being a higher level (OS level?) exception handler hook capability that will get invoked.

The down side is that the stack trace might be a bit overwhelming to the enduser (when there's no debugger attached), though the human readable error message would still be there.

Also: what about rt_term? Should we fflush stdout there? (In fact, the code in rt_term is nearly identical to the one in _d_run_main and I wonder why we don't use rt_term to begin with?)

alexrp and others added 15 commits June 19, 2013 06:53
Attempt to fix "Unable to set thread priority" bug
This reverts commit 7df7e30, reversing
changes made to 73d3205.
Mark `core.exception` functions with `pure`, `nothrow`, and friends.
Require user-supplied assert hander to be `nothrow`.

Issue URL: http://d.puremagic.com/issues/show_bug.cgi?id=10420
…ions-attributes

Fix Issue 10420 - Incorrect function attributes in `core.exception`
Fix issue #10436 - The runtime should print stack traces to stderr
…Info_AssociativeArray`

Override `equals` in `TypeInfo_AssociativeArray`.
… on failure

Flushing all open streams on exit, throwing an Error with the errno string in case of failure.
… on failure

Don't show errno, just the strerror.
… on failure

Flushing all open streams on exit, throwing an Error with the errno string in case of failure.
@lionello
Copy link
Contributor Author

I've clearly done something wrong here. I tried to rebase my fix on upstream/master, but now it looks like my PR pulls in all these other changes. Closing.

@lionello lionello closed this Jun 24, 2013
@andralex
Copy link
Member

OK, please do follow up with another diff. Thanks!

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

Successfully merging this pull request may close these issues.