JDK-6592751 : EmbeddedFrame disposal is fragile and breaks clean AppContext termination
  • Type: Bug
  • Component: client-libs
  • Sub-Component: java.awt
  • Affected Version: 6u10,7
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • OS: generic,windows
  • CPU: generic,x86
  • Submitted: 2007-08-14
  • Updated: 2011-05-18
  • Resolved: 2011-05-18
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availabitlity Release.

To download the current JDK release, click here.
JDK 6 JDK 7
6u10Resolved 7 b25Fixed
Related Reports
Duplicate :  
Relates :  
Relates :  
Relates :  
Relates :  
Relates :  
Description
At least on the Windows platform, the disposal of EmbeddedFrames (WEmbeddedFrames) is fragile and can lead to situations where the AppContext in which the WEmbeddedFrame was created can not be properly disposed.

If the holder of a WEmbeddedFrame destroys the containing window before WEmbeddedFrame.dispose() is called, the native peer will process the WM_DESTROY windows message and destroy and unlink the native peer from the Java-level WEmbeddedFrame object without giving it an indication that it has been destroyed. Subsequent calls to WEmbeddedFrame.dispose() will raise a NullPointerException. If an NPE is raised during AppContext.dispose() (the loop processing ownerless windows, at least in JDK 6 and later) then other top-level windows won't be properly disposed. The WEmbeddedFrame.dispose() code should be made more robust, as should the loop in AppContext.dispose(), since if any exception is raised during window disposal, other windows and resources won't be properly disposed. In the case of the Java Plug-In, this has the bad side-effect of potentially leaving applets' top-level windows on the screen after the applet has theoretically terminated.

Comments
EVALUATION since plugin really need us to not throw exception during disposal of embedded frame we should handle this unusual situation with receiving WM_DESTROY w/o calling dispose(). Thus it looks reasonable for me to add one more flag for WObjectPeer and set it when we do receive WM_DESTROY and set pData to NULL. And so do not throw NPE for such situations. This way we will still throw exception when pData is NULL, but we thinks that the native object was not destroyed.
2007-10-25

SUGGESTED FIX ------- awt_Object.h ------- *** /tmp/sccs.FOdX5M 2007-10-25 15:43:41.000000000 -0700 --- awt_Object.h 2007-10-25 15:21:46.000000000 -0700 *************** *** 51,56 **** --- 51,57 ---- /* sun.awt.windows.WObjectPeer field and method ids */ static jfieldID pDataID; + static jfieldID destroyedID; static jfieldID targetID; static jmethodID getPeerForTargetMID; ------- awt_Component.cpp ------- *** /tmp/sccs.hlbnzX 2007-10-25 15:43:41.000000000 -0700 --- awt_Component.cpp 2007-10-25 15:41:15.000000000 -0700 *************** *** 5399,5406 **** JNIEnv *env = (JNIEnv *)JNU_GetEnv(jvm, JNI_VERSION_1_2); if (m_peerObject) { env->SetLongField(m_peerObject, AwtComponent::hwndID, 0); ! JNI_SET_PDATA(m_peerObject, static_cast<PDATA>(NULL)); ! env->DeleteGlobalRef(m_peerObject); m_peerObject = NULL; } } --- 5399,5407 ---- JNIEnv *env = (JNIEnv *)JNU_GetEnv(jvm, JNI_VERSION_1_2); if (m_peerObject) { env->SetLongField(m_peerObject, AwtComponent::hwndID, 0); ! JNI_SET_PDATA(m_peerObject, static_cast<PDATA>(NULL)); ! JNI_SET_DESTROYED(m_peerObject); ! env->DeleteGlobalRef(m_peerObject); m_peerObject = NULL; } } ------- awt.h ------- *** /tmp/sccs.u7Kul8 2007-10-25 15:43:41.000000000 -0700 --- awt.h 2007-10-25 15:38:30.000000000 -0700 *************** *** 47,53 **** JNI_CHECK_NULL_GOTO(peer, "peer", where); \ pData = JNI_GET_PDATA(peer); \ if (pData == NULL) { \ ! JNU_ThrowNullPointerException(env, "null pData"); \ goto where; \ } \ } --- 47,56 ---- JNI_CHECK_NULL_GOTO(peer, "peer", where); \ pData = JNI_GET_PDATA(peer); \ if (pData == NULL) { \ ! jboolean destroyed = JNI_GET_DESTROYED(peer); \ ! if (destroyed != JNI_TRUE) { \ ! JNU_ThrowNullPointerException(env, "null pData"); \ ! } \ goto where; \ } \ } *************** *** 63,69 **** JNI_CHECK_NULL_RETURN(peer, "peer"); \ pData = JNI_GET_PDATA(peer); \ if (pData == NULL) { \ ! JNU_ThrowNullPointerException(env, "null pData"); \ return; \ } \ } --- 66,75 ---- JNI_CHECK_NULL_RETURN(peer, "peer"); \ pData = JNI_GET_PDATA(peer); \ if (pData == NULL) { \ ! jboolean destroyed = JNI_GET_DESTROYED(peer); \ ! if (destroyed != JNI_TRUE) { \ ! JNU_ThrowNullPointerException(env, "null pData"); \ ! } \ return; \ } \ } *************** *** 96,102 **** JNI_CHECK_NULL_RETURN_NULL(peer, "peer"); \ pData = JNI_GET_PDATA(peer); \ if (pData == NULL) { \ ! JNU_ThrowNullPointerException(env, "null pData"); \ return 0; \ } \ } --- 102,111 ---- JNI_CHECK_NULL_RETURN_NULL(peer, "peer"); \ pData = JNI_GET_PDATA(peer); \ if (pData == NULL) { \ ! jboolean destroyed = JNI_GET_DESTROYED(peer); \ ! if (destroyed != JNI_TRUE) { \ ! JNU_ThrowNullPointerException(env, "null pData"); \ ! } \ return 0; \ } \ } *************** *** 105,120 **** JNI_CHECK_NULL_RETURN_VAL(peer, "peer", val); \ pData = JNI_GET_PDATA(peer); \ if (pData == NULL) { \ ! JNU_ThrowNullPointerException(env, "null pData"); \ return val; \ } \ } #define JNI_GET_PDATA(peer) (PDATA) env->GetLongField(peer, AwtObject::pDataID) ! #define JNI_SET_PDATA(peer, data) env->SetLongField(peer, \ AwtObject::pDataID, \ (jlong)data) /* /NEW JNI */ /* --- 114,136 ---- JNI_CHECK_NULL_RETURN_VAL(peer, "peer", val); \ pData = JNI_GET_PDATA(peer); \ if (pData == NULL) { \ ! jboolean destroyed = JNI_GET_DESTROYED(peer); \ ! if (destroyed != JNI_TRUE) { \ ! JNU_ThrowNullPointerException(env, "null pData"); \ ! } \ return val; \ } \ } #define JNI_GET_PDATA(peer) (PDATA) env->GetLongField(peer, AwtObject::pDataID) + #define JNI_GET_DESTROYED(peer) env->GetBooleanField(peer, AwtObject::destroyedID) ! #define JNI_SET_PDATA(peer, data) env->SetLongField(peer, \ AwtObject::pDataID, \ (jlong)data) + #define JNI_SET_DESTROYED(peer) env->SetBooleanField(peer, \ + AwtObject::destroyedID, \ + JNI_TRUE) /* /NEW JNI */ /* ------- awt_Object.cpp ------- *** /tmp/sccs.DBji8i 2007-10-25 15:43:41.000000000 -0700 --- awt_Object.cpp 2007-10-25 15:23:31.000000000 -0700 *************** *** 39,44 **** --- 39,45 ---- */ jfieldID AwtObject::pDataID; + jfieldID AwtObject::destroyedID; jfieldID AwtObject::targetID; jclass AwtObject::wObjectPeerClass; jmethodID AwtObject::getPeerForTargetMID; *************** *** 223,228 **** --- 224,230 ---- AwtObject::wObjectPeerClass = (jclass)env->NewGlobalRef(cls); AwtObject::pDataID = env->GetFieldID(cls, "pData", "J"); + AwtObject::destroyedID = env->GetFieldID(cls, "destroyed", "Z"); AwtObject::targetID = env->GetFieldID(cls, "target", "Ljava/lang/Object;"); *************** *** 233,238 **** --- 235,241 ---- AwtObject::createErrorID = env->GetFieldID(cls, "createError", "Ljava/lang/Error;"); DASSERT(AwtObject::pDataID != NULL); + DASSERT(AwtObject::destroyedID != NULL); DASSERT(AwtObject::targetID != NULL); DASSERT(AwtObject::getPeerForTargetMID != NULL); DASSERT(AwtObject::createErrorID != NULL); ------- WObjectPeer.java ------- *** /tmp/sccs.fFmTKu 2007-10-25 15:43:58.000000000 -0700 --- WObjectPeer.java 2007-10-25 15:20:43.000000000 -0700 *************** *** 31,36 **** --- 31,37 ---- } long pData; // The Windows handle for the native widget. + boolean destroyed = false; Object target; // The associated AWT object. private volatile boolean disposed;
2007-10-25

EVALUATION I think that to make disposal of AppContext more robust we need to to wrap disposal of every window into try-catch block so one exception can not break disposal of the whole app context.
2007-10-20

SUGGESTED FIX Here is a possible simple decision: ------- AppContext.java ------- *** /tmp/sccs.TfyCHd 2007-10-20 03:36:03.000000000 +0400 --- AppContext.java 2007-10-20 03:34:38.000000000 +0400 *************** *** 40,45 **** --- 40,47 ---- import java.util.Map; import java.util.Set; import java.util.HashSet; + import java.util.logging.Level; + import java.util.logging.Logger; import java.beans.PropertyChangeSupport; import java.beans.PropertyChangeListener; *************** *** 127,132 **** --- 129,135 ---- * @version %I% %G% */ public final class AppContext { + private static final Logger log = Logger.getLogger("sun.awt.AppContext"); /* Since the contents of an AppContext are unique to each Java * session, this class should never be serialized. */ *************** *** 386,392 **** public void run() { Window[] windowsToDispose = Window.getOwnerlessWindows(); for (Window w : windowsToDispose) { ! w.dispose(); } AccessController.doPrivileged(new PrivilegedAction() { public Object run() { --- 389,401 ---- public void run() { Window[] windowsToDispose = Window.getOwnerlessWindows(); for (Window w : windowsToDispose) { ! try { ! w.dispose(); ! } catch (Throwable t) { ! if (log.isLoggable(Level.FINER)) { ! log.log(Level.FINER, "exception occured while disposing app context", t); ! } ! } } AccessController.doPrivileged(new PrivilegedAction() { public Object run() {
2007-10-20

EVALUATION AWT disposal code on Windows platform differs greatly from release to release, at least in the current 7.0 source I have no place where NPE may be thrown from. In any case, the following files should be inspected in 6u?: awt_Component.cpp, awt_Object.cpp, awt_Toolkit.cpp. AppContext.dispose() method should also be refactored.
2007-08-20