SUGGESTED FIX
--- old/src/share/classes/javax/swing/SwingWorker.java 2009-02-09 18:59:53.000000000 +0300
+++ new/src/share/classes/javax/swing/SwingWorker.java 2009-02-09 18:59:52.000000000 +0300
@@ -778,35 +778,34 @@
threadFactory);
appContext.put(SwingWorker.class, executorService);
- //register shutdown hook for this executor service
+ // Don't use ShutdownHook here as it's not enough. We should track
+ // AppContext disposal instead of JVM shutdown, see 6799345 for details
final ExecutorService es = executorService;
- final Runnable shutdownHook =
- new Runnable() {
- final WeakReference<ExecutorService> executorServiceRef =
- new WeakReference<ExecutorService>(es);
- public void run() {
- final ExecutorService executorService =
- executorServiceRef.get();
- if (executorService != null) {
- AccessController.doPrivileged(
- new PrivilegedAction<Void>() {
- public Void run() {
- executorService.shutdown();
- return null;
+ appContext.addPropertyChangeListener(AppContext.DISPOSED_PROPERTY_NAME,
+ new PropertyChangeListener() {
+ @Override
+ public void propertyChange(PropertyChangeEvent pce) {
+ boolean wasDisposed = (Boolean)pce.getOldValue();
+ boolean disposed = (Boolean)pce.getNewValue();
+ if (!wasDisposed && disposed) {
+ final WeakReference<ExecutorService> executorServiceRef =
+ new WeakReference<ExecutorService>(es);
+ final ExecutorService executorService =
+ executorServiceRef.get();
+ if (executorService != null) {
+ AccessController.doPrivileged(
+ new PrivilegedAction<Void>() {
+ public Void run() {
+ executorService.shutdown();
+ return null;
+ }
}
- });
+ );
+ }
}
}
- };
-
- AccessController.doPrivileged(
- new PrivilegedAction<Void>() {
- public Void run() {
- Runtime.getRuntime().addShutdownHook(
- new Thread(shutdownHook));
- return null;
- }
- });
+ }
+ );
}
return executorService;
}
--- old/src/share/classes/javax/swing/TimerQueue.java 2009-02-09 18:59:54.000000000 +0300
+++ new/src/share/classes/javax/swing/TimerQueue.java 2009-02-09 18:59:54.000000000 +0300
@@ -191,7 +191,10 @@
} finally {
timer.getLock().unlock();
}
- } catch (InterruptedException ignore) {
+ } catch (InterruptedException ie) {
+ // Shouldn't ignore InterruptedExceptions here, so AppContext
+ // is disposed gracefully, see 6799345 for details
+ break;
}
}
}
|
|
|
EVALUATION
There are 2 exceptions reported in the description, and it seems both are caused by different problems in Swing code.
First, javax.swing.TimerQueue.run() method silently ignores all the interruptions. I don't know why it is implemented this way, but it seems wrong to me, and every InterruptedException should result in a break statement.
Second exception occurs in javax.swing.SwingWorker code. This class uses ThreadPoolExecutor to spawn new worker threads, and even registers a ShutdownHook to shut it down correctly. Unfortunately, it is not enough as there may be several AppContexts in the application, and disposing one of them not necessarily leads to JVM shutdown. That's why the code from ShutdownHook should be refactored, for example, to listen to AppContext disposal.
See suggested fix for details.
|
|
|
EVALUATION
see comments
|
|
|
|
Java