JDK-4302235 : AWT bugs causing JVM crash found and fixed - asking for new release
  • Type: Bug
  • Component: client-libs
  • Sub-Component: java.awt
  • Affected Version: 1.2.2
  • Priority: P3
  • Status: Closed
  • Resolution: Duplicate
  • OS: windows_nt
  • CPU: x86
  • Submitted: 2000-01-03
  • Updated: 2000-01-16
  • Resolved: 2000-01-16
Related Reports
Duplicate :  
Description

Name: skT88420			Date: 01/03/2000


java version "1.2.2"
Classic VM (build JDK-1.2.2-001, native threads, symcjit)

This report is similar/identical to 4238890, 4252946, 4236062, 4200425.
Because the problem is not fixed in any publicly available release
(1.3beta has similar behaviour), we report it again with a suggested
fix, which seems to work successfully.

Problem description: a high rate of updates for an array of JTextFields
causes the JVM to crash with an access violation
 The error can be reproduced with the example code BugTest.java
given below (this is a reduced example from bug report 4238890).
The error is best reproduced on dual-CPU WinNT machines.
Solaris machines are not affected since the bugs are in the
Windows-AWT sources. Single CPU WinNT machines show the same behavior
but it takes much longer for the JVM to crash.

  Bugs:
1.) In the class AwtSharedDC the variable m_useCount is not properly
guarded against use from different threads (e.g. Attach and Detach
 at the same time).
 The count is sometimes not updated correctly, which leads to premature
disposal of the SharedDC!!
  Fixed with a shared lock.
2.) In the class AwtGraphics the destructor uses the shared DC without
locking it!! This is a problem when the garbage collector thread is
 disposing
the object, while there is a screen update.
  Fixed by acquiring the lock before resetting it.
3.) In the class AwtGraphics, GdiFlush was used in the wrong places.
GdiFlush has to be called after the thread finished all its operations
and not before, because GdiFlush is threadspecific.
  Fixed by adding GdiFlushes in the right places. (not tested yet how
important this fix is).

What we need:
- please verify the bugs
- integrate the fixes in the next JDK / JRE release
- provide Deutsche Bank with a patched version of the JRE 1.2 within
the next 5 business days.

===BugTest.java===CUT HERE===========================
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
import javax.swing.event.*;
import javax.swing.border.*;

class BugTest extends JFrame
	implements ActionListener
{ 
	private int counter=0;
	private Timer timer= null;
	private int rows=5;
	private int cols=5;
	private JTextField[] inTexts;

	public BugTest()
	{
		setTitle("JavaTest");
		setSize(1000, 400);
		addWindowListener(new WindowAdapter()
			{ public void windowClosing(WindowEvent e)
				{ System.exit(0);
				}
			} );
		
		JPanel loPanel1 = new JPanel();
		loPanel1.setLayout(new GridLayout(rows,cols,5,5));
				inTexts = new JTextField[rows*cols];
		
		for (int loIdx=0;loIdx<inTexts.length;loIdx++)
		{
			inTexts[loIdx] = new JTextField();
			
			inTexts[loIdx].setEditable(false);
			
inTexts[loIdx].setHorizontalAlignment(JTextField.CENTER);
			loPanel1.add(inTexts[loIdx]);
		}
		getContentPane().add(loPanel1);
		timer=new Timer	(1,this);
		timer.setRepeats(true);
		timer.start();
	}
	public void actionPerformed(ActionEvent evt)
	{
		for (int i=0;i<inTexts.length;i++)
		{
				inTexts[i].setText("Counter: " +(counter++));
		}
	}

	public static void main(String[] args)
	{
		BugTest loFrame = new BugTest();
		loFrame.show();
	}
}
(Review ID: 99508) 
======================================================================

Comments
WORK AROUND Name: skT88420 Date: 01/03/2000 There is no workaround, the source has to be patched. Please find below the diffs for awt_Graphics.h and awt_Graphics.cpp included in the awt source tree of JDK 1.2.2. Path is: src/win32/native/sun/windows. ===awt_Graphics.cpp.diff===CUT HERE=========================== *** awt_Graphics.cpp.old Tue Jun 29 03:22:48 1999 --- awt_Graphics.cpp Wed Dec 29 17:53:10 1999 *************** *** 124,127 **** --- 124,130 ---- ReleaseDDResources(); } + + // InvalidateDC called in Detach uses the DC without + // proper locking !!! if (m_pSharedDC != NULL) { m_pSharedDC->Detach(this); *************** *** 261,265 **** void AwtGraphics::InvalidateDC() { - ::GdiFlush(); SetPenColor(FALSE); SetBrushColor(FALSE); --- 264,267 ---- *************** *** 272,275 **** --- 274,279 ---- LogicalSelectClipRgn(m_hDC, NULL); } + // The operations must be flushed in the current thread!! + ::GdiFlush(); m_hDC = NULL; } *************** *** 461,465 **** return; COLORREF color = PALETTERGB(0xff, 0xff, 0xff); ! hBrush = AwtBrush::Get(color)->GetHandle(); } RECT rect; --- 465,469 ---- return; COLORREF color = PALETTERGB(0xff, 0xff, 0xff); ! hBrush = (HBRUSH)AwtBrush::Get(color)->GetHandle(); } RECT rect; *************** *** 1865,1869 **** * since we can't lock the object, we'll serialize access to this * method. */ - static CriticalSection detachLock; CriticalSection::Lock l(detachLock); if (pOldOwner == NULL) { --- 1869,1872 ---- *************** *** 1872,1879 **** } else if (m_pOwner == pOldOwner) { // Release this Device Context and leave it "ownerless" ! m_pOwner->InvalidateDC(); m_pOwner = NULL; } if (--m_useCount == 0) { delete this; } --- 1875,1892 ---- } else if (m_pOwner == pOldOwner) { // Release this Device Context and leave it "ownerless" ! ! //Make sure the DC is LOCKED when calling InvalidateDC ! // because this call might happen from a different thread ! // for example the destructor of AWTGraphics which has ! // no lock ! Lock(); ! m_pOwner->InvalidateDC(); ! Unlock(); m_pOwner = NULL; } if (--m_useCount == 0) { + // Potential problem if a new Graphics object + // Attaches itself in this very moment!!! + // Probably can't happen delete this; } ===awt_Graphics.h.diff===CUT HERE=========================== *** awt_Graphics.h.old Tue Jun 29 03:22:50 1999 --- awt_Graphics.h Wed Dec 29 17:43:24 1999 *************** *** 81,84 **** --- 81,90 ---- /* Attach this Shared DC to some object (increment reference count) */ INLINE void Attach() { + // Because the useCount is used by different threads it has to be + // inside its own CriticalSection. + // ++m_useCount is NOT necessarily atomic!!!! + // -> an increment can be forgotten + // m_useCount can get zero prematurely -> crash + CriticalSection::Lock l(detachLock); ++m_useCount; } *************** *** 131,134 **** --- 137,143 ---- AwtComponent *m_pComponent; CriticalSection m_lock; + //RJ This lock is used by Attach and Detach so it has to be + //a member variable + CriticalSection detachLock; #ifdef DEBUG *************** *** 272,279 **** } } catch (...) { m_pSharedDC->Unlock(); throw; } ! m_pSharedDC->Unlock(); } } --- 281,291 ---- } } catch (...) { + ::GdiFlush(); m_pSharedDC->Unlock(); throw; } ! // When finished flush the operations!!! ! ::GdiFlush(); ! m_pSharedDC->Unlock(); } } ======================================================================
11-06-2004

EVALUATION I believe we have already fixed these crashes in delta 1.143.1.2 of awt_Graphics.cpp. Additional deadlock issues were resolved in delta 1.148. These fixes will be available in the FCS release of kestrel. I will leave this bug open for now in case the submitter wants to open an escalation against 1.2.2. david.mendenhall@eng 2000-01-05
05-01-2000