Review request 6985422: flush the output streams before OnError commands

[David Holmes]

Chuck Rasbold said the following on 09/18/10 03:07:
> On Thu, Sep 16, 2010 at 4:57 PM, David Holmes <David.Holmes at oracle.com 
> <mailto:David.Holmes at oracle.com>> wrote:
>     Why was the ostream_abort() removed from the os::shutdown() path?
> 
>     In the existing code we have:
> 
>     VMError::report_and_die()
>      -> os::abort()
>        -> os::shutdown()
>           -> ostream_abort()
> 
> 
> It seems backward to me the the os-specific abort() calls ostream_abort(). 
> 
> Ostream_init() and ostream_init_log() are called from create_vm(), I 
> think it would be preferable to clean-up the ostreams at a similar level.

I don't see how you can given that you may need the streams right up 
until you actually abort the process, and that is done down in the 
depths of the os::abort/shutdown code at the end of the error path. 
Normal VM termination is a different matter and ostream_exit() is used 
for that purpose.

>     and you've moved ostream_abort() into report_and_die(), but there
>     are other exit paths that also call os::shutdown() and will no
>     longer call ostream_abort():
> 
>     vm_shutdown_during_initialization()
>     -> vm_shutdown()
>        -> os::shutdown()
> 
> 
> Agreed that I missed vm_shutdown() in java.cpp.   I'd be happy to add a 
> call to ostream_abort() there.

I think this is the wrong place to "abort" the output streams. It only 
works, in my view, because the tty and gc streams don't actually get 
"aborted" just flushed. Otherwise I suspect that there might be use of 
those streams after the current call to ostream_abort().

> Are there other call sites to os:shutdown() that I've missed?  If so, 
> perhaps it is safer to leave the originals calls in os::abort() as I've 
> underestimated the complexity of the shutdown code.

I don't think you missed anything else, but I think this is the wrong 
way to fix it.

>     Also with this move you need to fix this comment in ostream.cpp:
> 
>     // ostream_abort() is called by os::abort() when VM is about to die.
>     void ostream_abort() {
> 
> True.
>  
> 
>     Why not just add explicit flush() calls where needed?
> 
> 
> Flushing is pretty much what ostream_abort() does.  The compilation log 
> flushing code is short but non-trivial.  I think it is best not 
> duplicate the it.

I would rather see distinct flushing and "aborting" methods for the 
streams. ostream_abort() happens to also need to perform flushing, but 
flushing need not lead to aborting.

David

> -- Chuck
> 
> 
>     David Holmes
> 
> 
>         Contributed-by: rasbold at google.com <mailto:rasbold at google.com>
> 
>