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
