United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6592751 EmbeddedFrame disposal is fragile and breaks clean AppContext termination
JDK-6592751 : EmbeddedFrame disposal is fragile and breaks clean AppContext termination

Details
Type:
Bug
Submit Date:
2007-08-14
Status:
Closed
Updated Date:
2011-05-18
Project Name:
JDK
Resolved Date:
2011-05-18
Component:
client-libs
OS:
generic,windows
Sub-Component:
java.awt
CPU:
x86,generic
Priority:
P2
Resolution:
Fixed
Affected Versions:
6u10,7
Fixed Versions:

Related Reports
Backport:
Duplicate:
Relates:
Relates:
Relates:
Relates:
Relates:

Sub Tasks

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

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

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

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



Hardware and Software, Engineered to Work Together