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 Availability Release.

To download the current JDK release, click here.
JDK 6 JDK 7
6u10Fixed 7 b25Fixed
Related Reports
Duplicate :  
JDK-6638153 - Java Console crash with new OOPP
Relates :  
JDK-6605126 - Add detection of failed applet shutdown and notion of tainting
Relates :  
JDK-6609271 - Plugin2: ClassNotFound exception getting thrown sometimes once applet is refreshed
Relates :  
JDK-6627386 - JavaScript errors with gamehouse.com applets
Relates :  
JDK-6616692 - Two Java consoles exist when refresh the game applets
Relates :  
JDK-6613949 - run Yokogawa reliability test with new plug-in bundle caused big handle leaks
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
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;
25-10-2007
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.
25-10-2007
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() {
20-10-2007
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.
20-10-2007
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.
20-08-2007