Review request for 6829503

David Holmes - Sun Microsystems David.Holmes at Sun.COM
Sat Apr 18 08:09:55 UTC 2009

Now that I understand what this is for I see the predicament. Basically 
there's no single correct action because it all depends on what the 
application was doing, and will do, during shutdown.

The current behaviour handles the most likely case of hitting ctrl-c 
while the password is being prompted for.

Running the console hook before and after the application hook, would be 
slightly "safer" but I don't think it's really worth the effort.

Seems to me the fundamental problem here is the semantics for concurrent 
use of the Console ... but that's a different issue.

So I have no issue with the fix as-is.


Xueming Shen said the following on 04/18/09 16:51:
> Mandy Chung wrote:
>> 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.
> Two use scenarios
> (1) jvm is being terminated "abnormally" during a password reading 
> operation (during "normal" application
> operation), in which the echo is in "off" mode when these shutdown hooks 
> start to be invoked. Without
> the "console restore first", application hooks probably can do nothing 
> with the stdin/console
> (2)Application hooks use Console.readPassword() and then be terminated 
> "abnormally" during the reading,
> in which the echo mode is kept in "off" mode and will not be restored 
> after app exits.
> The reason the console hook was/is added to run first is we weighed the 
> first scenario as a more likely
> scenario in real world application. Note, the Console echo mode is not 
> required to be "restored" if the
> reading operation finishes normally.
> Maybe we can consider to run the console hook again (if the hook is set) 
> after the application hooks?
> Sherman
>> 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:
>>>> 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 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

More information about the core-libs-dev mailing list