Review request for 6829503

David Holmes - Sun Microsystems David.Holmes at Sun.COM
Sat Apr 18 01:14:44 UTC 2009

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).


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