Review request for 6829503

[Mandy Chung]

David,

Thanks for the review.

Whether the application shutdown hooks should always be invoked first is 
a good question.  

The Console shutdown hook is added to restore the console after 
prompting for a password to fix:
  6363043 Console will not return to original state when the process is 
killed with Ctrl+C (sol)

I consulted with Sherman some time back about the shutdown hook 
ordering.  The Console restores the state before the application 
shutdown hook is to enable the application shutdown hooks that use 
Console to work properly.

While working on this fix, I also observed the issue you point out - we 
won't restore the state of the console if the first use of the console 
is by an application shutdown or by some other thread when shutdown has 
commenced.   It is only an issue when it gets terminated in the middle 
of reading the password.   Normal return of the Console.readPassword 
method will restore the state.

I consulted with Alan.   He points out that it would go against all 
recommendations to prompt for a password in a shutdown hook.  He 
suggests to leave the existing behavior as it is.

Alan, Sherman,
    Do you have any comments on the order of Console shutdown hook and 
application shutdown hooks?  It seems sensible that the application 
shutdown hooks should run first and then other "internal hooks" as David 
said.

Thanks
Mandy

David Holmes - Sun Microsystems wrote:
> Hi Mandy,
>
> Looks good but I have one query.
>
> At the top-level there are 3 shutdown hooks:
>
> - console hook
> - application hooks
> - deleteOnExit hook
>
> and they run in this order. The deleteOnExit hook can be added when 
> shutdown is in progress, so this allows first-use of deleteOnExit 
> during an application shutdown hook. Okay - that's fine and solves 
> current problem very neatly.
>
> But the Console hook that restores the echo state runs first, so if 
> the first use of the console is by an application shutdown hook, or by 
> some other thread when shutdown has commenced, then we won't restore 
> the state of the console. I guess that's how it currently works 
> anyway, but it seems strange. I'm not familiar with the Console but I 
> would have expected (naively perhaps) the application shutdown hooks 
> to run first and then any "internal hooks". That way we would always 
> restore the state (though I guess there is potentially a race with 
> active threads still using the console).
>
> David
>
> Mandy Chung said the following on 04/18/09 08:05:
>> 6829503: addShutdownHook fails if called after shutdown has commenced.
>>
>> Webrev at:
>>   http://cr.openjdk.java.net/~mchung/6829503/webrev.00/
>>
>> I change the Shutdown#add method to take the 
>> registerShutdownInProgress parameter.  If set to true, the specified 
>> shutdown hook is allowed to be registered while shutdown is in 
>> progress.  The method will throw IllegalStateException if the 
>> shutdown process already passes this slot.   DeleteOnExitHook is the 
>> last shutdown hook to be invoked and it will not be invoked until all 
>> application shutdown hooks finish (see 
>> ApplicationShutdownHooks.runHooks()).  So any file added to the 
>> delete on exit list by the application shutdown hooks will be handled 
>> by the DeleteOnExitHook.
>>
>> The LoggingDeadlock2.java test passes with this fix.  I also add a 
>> new jtreg test to exercise the Console and DeleteOnExitHook being 
>> initialized during application shutdown.
>>
>> Alan,
>>   I considered your suggestion to make Shutdown#add method to return 
>> a boolean instead of checking the state.  I am concerned that if the 
>> caller didn't check the return value and handle properly, it would be 
>> harder to catch the problem.  So I keep it to check the state and 
>> throw IllegalStateException.
>>
>> Thanks
>> Mandy