JDK-5044798 : Significant memory leak in BasicListUI: Renderer not removed from renderer pane
  • Type: Bug
  • Component: client-libs
  • Sub-Component: javax.swing
  • Affected Version: 1.4.2,6
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: linux,windows_2000,windows_xp
  • CPU: x86
  • Submitted: 2004-05-10
  • Updated: 2004-11-03
  • Resolved: 2004-09-22
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
6 betaFixed
Related Reports
Duplicate :  
Relates :  
Relates :  
Description
Name: gm110360			Date: 05/10/2004


FULL PRODUCT VERSION :
java version "1.4.2_03"
Java(TM) 2 Runtime Environment, Standard Edition (build 1.4.2_03-b02)
Java HotSpot(TM) Client VM (build 1.4.2_03-b02, mixed mode)


ADDITIONAL OS VERSION INFORMATION :
Windows NT Version 4.0

A DESCRIPTION OF THE PROBLEM :
BasicListUI uses a CellRenderer pane to paint the renderer component
retrieved via getListCellRenderer from the JList instance. CellRendererPane paint the component but it never removes it from its component hierarchy.

This has huge impact. Jlist can produce dynamic renderer component instances in getListCellRenderer method. Each one of these instances
will linger in memory because it remains in the component hierarchy of the CellRendererPane.

Methods with a bug in BasicListUI:

protected void updateLayoutState()
protected void paintCell()

This bug is present in all 1.3.x versions and 1.4.x versions

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Inspect the source code! It's that simple. Look at javax.swing.plaf.basic.BasicListUI.java

The original code:

/**
     * Paint one List cell: compute the relevant state, get the "rubber stamp"
     * cell renderer component, and then use the CellRendererPane to paint it.
     * Subclasses may want to override this method rather than paint().
     *
     * @see #paint
     */
    protected void paintCell(
        Graphics g,
        int row,
        Rectangle rowBounds,
        ListCellRenderer cellRenderer,
        ListModel dataModel,
        ListSelectionModel selModel,
        int leadIndex)
    {
        Object value = dataModel.getElementAt(row);
        boolean cellHasFocus = list.hasFocus() && (row == leadIndex);
        boolean isSelected = selModel.isSelectedIndex(row);

        Component rendererComponent =
            cellRenderer.getListCellRendererComponent(list, value, row, isSelected, cellHasFocus);

        int cx = rowBounds.x;
        int cy = rowBounds.y;
        int cw = rowBounds.width;
        int ch = rowBounds.height;
        rendererPane.paintComponent(g, rendererComponent, list, cx, cy, cw, ch, true);

       ///////////////// BUG ///////////// rendererComponent is never removed here
    }




protected void updateLayoutState()
    {
        /* If both JList fixedCellWidth and fixedCellHeight have been
         * set, then initialize cellWidth and cellHeight, and set
         * cellHeights to null.
         */

        int fixedCellHeight = list.getFixedCellHeight();
        int fixedCellWidth = list.getFixedCellWidth();

        cellWidth = (fixedCellWidth != -1) ? fixedCellWidth : -1;

        if (fixedCellHeight != -1) {
            cellHeight = fixedCellHeight;
            cellHeights = null;
        }
        else {
            cellHeight = -1;
            cellHeights = new int[list.getModel().getSize()];
        }

        /* If either of  JList fixedCellWidth and fixedCellHeight haven't
         * been set, then initialize cellWidth and cellHeights by
         * scanning through the entire model.  Note: if the renderer is
         * null, we just set cellWidth and cellHeights[*] to zero,
         * if they're not set already.
         */

        if ((fixedCellWidth == -1) || (fixedCellHeight == -1)) {

            ListModel dataModel = list.getModel();
            int dataModelSize = dataModel.getSize();
            ListCellRenderer renderer = list.getCellRenderer();

            if (renderer != null) {
                for(int index = 0; index < dataModelSize; index++) {
                    Object value = dataModel.getElementAt(index);
                    Component c = renderer.getListCellRendererComponent(list, value, index, false, false);
                    rendererPane.add(c);
                    Dimension cellSize = c.getPreferredSize();
                    if (fixedCellWidth == -1) {
                        cellWidth = Math.max(cellSize.width, cellWidth);
                    }
                    if (fixedCellHeight == -1) {
                        cellHeights[index] = cellSize.height;
                    }

                   /// BUG /// Component c which is the renderer component is added but never removed from rendererPane
                }
            }
            else {
                if (cellWidth == -1) {
                    cellWidth = 0;
                }
                if (cellHeights == null) {
                    cellHeights = new int[dataModelSize];
                }
                for(int index = 0; index < dataModelSize; index++) {
                    cellHeights[index] = 0;
                }
            }
        }

        columnCount = 1;
        if (layoutOrientation != JList.VERTICAL) {
            updateHorizontalLayoutState(fixedCellWidth, fixedCellHeight);
        }
    }




EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
The renderer component gets removed from the component hierarchy of the CellRendererPane after its painting is done.
ACTUAL -
ALL instances of a renderer linger in memory and are never garbage collected

REPRODUCIBILITY :
This bug can be reproduced always.

CUSTOMER SUBMITTED WORKAROUND :
The fix is simple. Add a line of code to each one of the buggy methods
as done below with the ///FIX comment.


protected void updateLayoutState()
    {
        /* If both JList fixedCellWidth and fixedCellHeight have been
         * set, then initialize cellWidth and cellHeight, and set
         * cellHeights to null.
         */

        int fixedCellHeight = list.getFixedCellHeight();
        int fixedCellWidth = list.getFixedCellWidth();

        cellWidth = (fixedCellWidth != -1) ? fixedCellWidth : -1;

        if (fixedCellHeight != -1) {
            cellHeight = fixedCellHeight;
            cellHeights = null;
        }
        else {
            cellHeight = -1;
            cellHeights = new int[list.getModel().getSize()];
        }

        /* If either of  JList fixedCellWidth and fixedCellHeight haven't
         * been set, then initialize cellWidth and cellHeights by
         * scanning through the entire model.  Note: if the renderer is
         * null, we just set cellWidth and cellHeights[*] to zero,
         * if they're not set already.
         */

        if ((fixedCellWidth == -1) || (fixedCellHeight == -1)) {

            ListModel dataModel = list.getModel();
            int dataModelSize = dataModel.getSize();
            ListCellRenderer renderer = list.getCellRenderer();

            if (renderer != null) {
                for(int index = 0; index < dataModelSize; index++) {
                    Object value = dataModel.getElementAt(index);
                    Component c = renderer.getListCellRendererComponent(list, value, index, false, false);
                    rendererPane.add(c);
                    Dimension cellSize = c.getPreferredSize();
                    if (fixedCellWidth == -1) {
                        cellWidth = Math.max(cellSize.width, cellWidth);
                    }
                    if (fixedCellHeight == -1) {
                        cellHeights[index] = cellSize.height;
                    }

                    // FIX
                   rendererPane.remove(c);
                }
            }
            else {
                if (cellWidth == -1) {
                    cellWidth = 0;
                }
                if (cellHeights == null) {
                    cellHeights = new int[dataModelSize];
                }
                for(int index = 0; index < dataModelSize; index++) {
                    cellHeights[index] = 0;
                }
            }
        }

        columnCount = 1;
        if (layoutOrientation != JList.VERTICAL) {
            updateHorizontalLayoutState(fixedCellWidth, fixedCellHeight);
        }
    }



protected void paintCell(
        Graphics g,
        int row,
        Rectangle rowBounds,
        ListCellRenderer cellRenderer,
        ListModel dataModel,
        ListSelectionModel selModel,
        int leadIndex)
    {
        Object value = dataModel.getElementAt(row);
        boolean cellHasFocus = list.hasFocus() && (row == leadIndex);
        boolean isSelected = selModel.isSelectedIndex(row);

        Component rendererComponent =
            cellRenderer.getListCellRendererComponent(list, value, row, isSelected, cellHasFocus);

        int cx = rowBounds.x;
        int cy = rowBounds.y;
        int cw = rowBounds.width;
        int ch = rowBounds.height;
        rendererPane.paintComponent(g, rendererComponent, list, cx, cy, cw, ch, true);

       // FIX
      rendererPane.remove(rendererComponent);
    }
(Incident Review ID: 237018) 
======================================================================
###@###.### 11/3/04 16:32 GMT

Comments
CONVERTED DATA BugTraq+ Release Management Values COMMIT TO FIX: mustang FIXED IN: mustang INTEGRATED IN: mustang
23-09-2004

WORK AROUND Subclass JList and invoke removeAll after painting is done: private CellRendererPane renderer; private CellRendererPane getRenderer() { if (renderer == null) { for (Component c : getComponents()) { if (c instanceof CellRendererPane) { renderer = (CellRendererPane)c; break; } } } return renderer; } protected void paintComponent(Graphics g) { super.paintComponent(g); CellRendererPane renderer = getRenderer(); if (renderer != null) { renderer.removeAll(); } } }; ###@###.### 2004-06-21
21-06-2004

EVALUATION Name: sh120115 Date: 05/12/2004 ###@###.###, I thought you might want first crack at evaluating this performance/painting issue. It looks like we don't remove things from cell renderer panes anywhere (except when uninstalling UIs). For example, in BasicTreeUI: // Only ever removed when UI changes, this is OK! rendererPane.add(aComponent); ###@###.### 2004-05-12 ====================================================================== BasicListUI needs a: rendererPane.removeAll(); after painting. ###@###.### 2004-05-13
12-05-2004