Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-4068620

hiding and then showing modal dialogs have race conditions causing crash/assert

    Details

    • Subcomponent:
    • Resolved In Build:
      1.1.5
    • CPU:
      x86
    • OS:
      windows_nt
    • Verification:
      Verified

      Description


      bug description from Jeffrey Melnic(Oracle) email:
      ==========================================================

      The awt_Dialog implementation still has several race conditions. This
      alleviates some of them, however a complete solution for the problem of
      hiding and then showing modal dialogs is deferred to AWT.
      The problems are as follows:
                                                
        1) If you hide a dialog and then show a dialog, the re-enabling and
           disabling of the underlying windows can assert in awt_dialog.
           The problem is that the re-enabling of the windows is run in the
           Window thread, and the counting and subsequent disabling of the
           windows is done in the thread showing the dialog -- worse, the
           counting and disabling are not atomic, and hence you can get into a
           situation where you've counted two windows, the Window thread
           re-enables another one, and then the EnumThreadWindows() call
           tries to disable three windows, and asserts.
           I've worked around the assert by re-allocating the array rather than
           asserting, however that still leaves the possibility that a window
           that *should* have been disabled, in reality, won't be -- because it's
           enabledness is deferred. It would be better to handle all of this in
           the Window thread during the WM_SHOW of the dialog, but we're going to
           defer to AWT on this one, as it isn't a crashing situation.
      The new AwtDialog::DisableHWnds() now looks like:
        BOOL CALLBACK AwtDialog::DisableHWnds(HWND hWnd, AwtDialog* dlg)
        {
          ASSERT(dlg->m_disabledWnds != NULL);
          if (!(::GetWindowLong(hWnd, GWL_STYLE) & WS_CHILD)) {
              if (hWnd != dlg->m_modalWnd && ::IsWindowEnabled(hWnd)) {
                  ASSERT(dlg->m_iWnd < dlg->m_nWnds);
                  if ( dlg->m_iWnd >= dlg->m_nWnds )
                  {
                      HWND *temp = new HWND[dlg->m_nWnds + 10];
                      ASSERT(temp != NULL);
                      ASSERT(temp != NULL);
                      memcpy(temp, dlg->m_disabledWnds,
                             sizeof(*temp) * dlg->m_nWnds);
                      dlg->m_nWnds += 10;
                      delete[] dlg->m_disabledWnds;
                      dlg->m_disabledWnds = temp;
                  }
                  dlg->m_disabledWnds[dlg->m_iWnd++] = hWnd;
                  ::EnableWindow(hWnd, FALSE);
              }
          }
          return TRUE;
        }
      .
           I additionally no longer count the windows up front, as you will see in
           the code for ShowModal() below 2) and 3).
      .
        2) The second problem was a nuisance, but I fixed while I was in this code.
           The Hide() call in EndModal() was happening before the calls to
           re-enable the underlying windows, which means that focus could not
           be given to any of the underlying windows. Most often, that meant that
           the DOS prompt got focus. This was annoying, but I appear to have
           fixed that by moving Hide() *after* the call to re-enable the windows.
        3) The third problem seems to indicate a strangeness within the
           implementation of ShowWindow(FALSE). While this call was taking place,
           the window actually failed to hide. I *believe* that the problem was
           that the modal dialog's thread was, at the same time, attempting to
           call SetStyle() and SetWindowLong(). Somehow, the simultaneity of
           these caused ShowWindow() to fail.
           I moved these calls into the EndModal() code right before the call to
           Hide() so that I knew the window wasn't being hidden at that point,
           and our very intermittent problem of leftover modal dialogs seems to
           have disappeared.
      .
      Rather than giving complex diffs to ShowModal and EndModal(), I include
      both calls below:
      void AwtDialog::ShowModal()
      {
          m_previousModalDialog = AwtToolkit::GetInstance().SetModal(this);
      .
          // Yes, indeedy, we are modal.
          m_modalWnd = GetHWnd();
      .
          // disable all other frames created by this thread.
          /*
          ** Keep this old code around, just in case....
          ** We want DisableHWnds() to be done in the Window thread, really,
          ** see notes at end of EndModal().
          m_nWnds = 0;
          ::EnumThreadWindows(AwtToolkit::MainThread(), (WNDENUMPROC)CountHWnds,
                              (LPARAM)this);
          m_disabledWnds = new HWND[m_nWnds];
          ASSERT(m_disabledWnds);
          m_iWnd = 0;
          ::EnumThreadWindows(AwtToolkit::MainThread(), (WNDENUMPROC)DisableHWnds,
                              (LPARAM)this);
          **
          */
      .
          // Assume ten windows to start with (hey, why not).
          m_nWnds = 10;
          m_disabledWnds = new HWND[m_nWnds];
          m_iWnd = 0;
          ::EnumThreadWindows(AwtToolkit::MainThread(), (WNDENUMPROC)DisableHWnds,
                              (LPARAM)this);
          // Reset the number of windows to the counter returned from
          // DisableHWnds in m_iWnd.
          m_nWnds = m_iWnd;
      .
          // Set modal styles before showing to get the best windows modal look
          SetStyle(GetStyle() | DS_MODALFRAME);
          if (IS_WIN4X) {
              DWORD exStyle =
                  ::GetWindowLong(GetHWnd(), GWL_EXSTYLE) | WS_EX_DLGMODALFRAME;
              ::SetWindowLong(GetHWnd(), GWL_EXSTYLE, exStyle);
          }
      .
          ::EnableWindow(GetHWnd(), TRUE);
          Show();
          ::SetForegroundWindow(GetHWnd());
      .
          // Two methods of blocking must be used, depending upon whether the
          // caller is on our main thread. If so, we need to start a
          // secondary message pump, which will block the caller while still
          // handling outgoing messages. If not, just wait on the object's
          // monitor.
          m_onMainThread =
              (AwtToolkit::MainThread() == ::GetCurrentThreadId());
          if (m_onMainThread) {
              // This won't exit until StopMessagePump() is called in EndModal.
              AwtToolkit::GetInstance().MessageLoop();
          } else {
              Hsun_awt_windows_WDialogPeer* peer = GetPeer();
      .
              monitorEnter(obj_monitor(peer));
              monitorWait(obj_monitor(peer), -1);
              monitorExit(obj_monitor(peer));
          }
      .
          // The Styles are now reset in EndModal
      }
      .
      void AwtDialog::EndModal()
      {
          // Javasoft fix.
          if ( AwtToolkit::GetInstance().GetModalDialog() != this ) {
              return;
          }
          if ((GetStyle() & WS_VISIBLE) == 0) {
              return;
          }
      .
      .
          //DCN
          if ( !m_modalWnd )
              return;
          m_modalWnd = NULL;
      .
          AwtToolkit::GetInstance().ResetModal(m_previousModalDialog);
      .
          // Enable all previously disabled windows.
          if (m_disabledWnds != NULL) {
              for (int i = 0; i < m_nWnds; i++) {
                  // Post "enable" messages, since this object is locked.
                  // This is necessary because one of them may yank focus from
                  // this object, causing a FocusEvent to be fired -- posting
                  // that event requires this component's lock.
                  ::PostMessage(m_disabledWnds[i], WM_AWT_COMPONENT_ENABLE, TRUE, 0);
              }
              delete[] m_disabledWnds;
              m_disabledWnds = NULL;
              m_nWnds = 0;
          }
      .
          // Remove modal styles set before show, incase next show is non-modal
          SetStyle(GetStyle() & ~DS_MODALFRAME);
          if (IS_WIN4X) {
              DWORD exStyle = ::GetWindowLong(GetHWnd(), GWL_EXSTYLE) &
                              ~WS_EX_DLGMODALFRAME;
              ::SetWindowLong(GetHWnd(), GWL_EXSTYLE, exStyle);
          }
      .
          // Must call hide() last so that the windows are re-enabled and so that
          // the Style has been set back. Note that the comment for the SetStyle
          // is wrong -- I copied as is from ShowModal(), and kept it before the
          // Hide() which seems safest for now. Hide() also, btw, indirects the
          // WM_SHOW via a Post to the Window thread. In reality, the
          // re-enabling of the windows and the resetting of the style should be
          // done THERE in response to the WM_SHOW event. The disabling would
          // similarly be done in response to the WM_SHOW event. This would
          // eliminate strange race conditions for enabling/disabling windows.
          // It's questionable as to whether the call to SetForegroundWindow()
          // should also be deferred (for both show() and hide()).
          // Defer fix to AWT.
          Hide();
      .
          if (m_onMainThread) {
              AwtToolkit::GetInstance().StopMessagePump();
          } else {
              Hsun_awt_windows_WDialogPeer* peer = GetPeer();
              monitorEnter(obj_monitor(peer));
              monitorNotifyAll(obj_monitor(peer));
              monitorExit(obj_monitor(peer));
          }
      .
          if (m_previousModalDialog != NULL) {
              ::SetForegroundWindow(m_previousModalDialog->GetHWnd());
              m_previousModalDialog = NULL;
          } else {
              AwtComponent* parent = GetParent();
              if (parent != NULL) {
                  ::SetForegroundWindow(parent->GetHWnd());
              }
          }
      }

        Attachments

          Activity

            People

            • Assignee:
              duke J. Duke (Inactive)
              Reporter:
              tyao Ting-Yun Ingrid Yao (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Imported:
                Indexed: