Bug 321135

Comment 1 Benjamin Muskalla 2010-07-28 11:02:12 EDT

Created attachment 175414 [details]
testcase

.

Comment 2 Ivan Furnadjiev 2011-04-13 06:19:06 EDT

I think that we have to handle this issue in a more generic way. We have a common pattern (in most of the widgets) to measure the string dimensions like:
Graphics.stringExtent( getFont(), text );
Graphics.textExtent( getFont(), text, wrapWidth );
Graphics.getCharHeight( getFont() )
....
With the possibility to dispose the font, not created by factory, we can easy run into same exception in other widgets. One possible solution could be to change the Control#getFont() from:
...
if( result == null ) {
...
to:
...
if( result == null || result.isDisposed() ) {
...
What do you think?

Comment 3 Ivan Furnadjiev 2011-04-13 06:36:56 EDT

BTW in SWT when you dispose a font the widget continue to work without exception even the getFont() returns the disposed font:
final Button button = new Button( parent, SWT.PUSH );
button.setLayoutData( new GridData( SWT.BEGINNING, SWT.CENTER, false, false ) );
button.setText( "Test button" );
Font font = new Font( parent.getDisplay(), "Arial", 20, SWT.BOLD );
button.setFont( font );
button.addSelectionListener( new SelectionAdapter() {
  public void widgetSelected( SelectionEvent e ) {
    Font font = button.getFont();
    System.out.println( font );
    font.dispose();
  }
} );

Comment 4 Rüdiger Herrmann 2011-04-13 08:15:01 EDT

Do you know what happens in SWT if the the disposed font/color/image is needed by the window system, e.g. in case of a repaint?

Comment 5 Ivan Furnadjiev 2011-04-13 09:06:19 EDT

I did some more testing with SWT (Windows) Button/Label and font/color/image set on it and disposed.
1. Font - after the disposal, visually the font is changed to some default font, but the method getFont() returns the disposed font. Doing redraw or layout works without exception.
2. Color - after the disposal, visually the same color is still in use and the method getForeground/Background returns equal undisposed color (safe copy). Doing redraw or layout works without exception.
3. Image - after the disposal, visually the same image is still in use, but the method getImage() returns the disposed image. Doing redraw works fine. Doing layout throws an exception "Graphics is disposed" in image#getBounds (expected as image is disposed).

Comment 6 Rüdiger Herrmann 2011-04-13 16:59:14 EDT

I am astonished at how tolerant SWT (at least on Windows) handles disposed of fonts, colors etc. However, RWT should not try to be such tolerant. IMHO, the use case at hand is valid, but anything beyond that isn't.

The source of the probelm seems to be that Table#destroyItem() updates topIndex, scrollBars etc. even though the whole table is disposed. This in turn seems to trigger the text size calculations. The fix would then be to avoid these updates if the table is diposed at once.

We should check the Tree (and maybe other widgets with items?) as well.

Comment 7 Ivan Furnadjiev 2011-04-14 03:04:45 EDT

Yes... I understand where is the exact problem described here and it's possible to find a fix (or I can consider it as workaround :-)) like you point out. But... I'm just thinking that in case of disposed fonts we can easy run into trouble in Table or other widgets. For example these test cases succeed in SWT but failed in RAP with "Graphics is disposed exception":
public void testGetItemHeightOnDisposedFont() {
    Display display = new Display();
    Shell shell = new Shell( display );
    Table table = new Table( shell, SWT.SINGLE | SWT.FULL_SELECTION );
    Font font = new Font( display, "Arial", 30, SWT.BOLD );
    table.setFont( font );
    font.dispose();
    table.getItemHeight();
  }
or
public void testDisposeColumnOnDisposedFont() {
    Display display = new Display();
    Shell shell = new Shell( display );
    Table table = new Table( shell, SWT.SINGLE | SWT.FULL_SELECTION );
    TableColumn column = new TableColumn( table, SWT.NONE );
    Font font = new Font( display, "Arial", 30, SWT.BOLD );
    table.setFont( font );
    font.dispose();
    column.dispose();
  }
  After all this testing in SWT I think that the fix proposed in comment#2 will fit better, will have a generic (cross widget) impact and the most important will make RAP and SWT to behave the same (without exception) in case of single-sourcing. The only difference will be the font object returned by Control#getFont() - disposed font in SWT, default font in RAP. But I think this is rather a problem of SWT as actually the font used by the widget is a default font and not the disposed one.

Comment 8 Ivan Furnadjiev 2011-04-15 04:45:34 EDT

Similar problem, but with disposed image in this bug 320201.

Comment 9 Rüdiger Herrmann 2011-04-15 05:04:47 EDT

Sadly, the change outlined in comment#2 isn't an option: The following test succeeds in SWT:
  public void testGetFontAfterDisposingFont() {
    Control control = new Label( shell, SWT.NONE );
    Font font = new Font( display, "font-name", 10, SWT.NONE );
    control.setFont( font );
    font.dispose();
    Font returnedFont = control.getFont();
    assertSame( font, returnedFont );
  }

As far as I understand, the proposed change would let this test fail.
So, what remains is to go for the changes outlined in comment #6, or are there any other options?

Comment 10 Rüdiger Herrmann 2011-04-16 08:19:43 EDT

Comment #3 in bug 342943 states that disposing of a resource while it is in use is a programming error and - while not explicitly checked - not supported in SWT.
Given this, I think we should fix the Table (and probably other widgets that manage Items) so that it does not recalculate scroll bars etc while being disposed. Or did I miss something?

Comment 11 Ivan Furnadjiev 2011-04-16 13:39:30 EDT

I'm a little bit confused now. Based on the example in comment #3 in bug 342943 I understand that all resources *must* be disposed after all widgets that are using these resources are disposed first. Thus, the statement:
font.dispose();
widget.dispose();
is illegal too and this bug is in general invalid. Am I understanding the comment #3 right?

Comment 12 Ivan Furnadjiev 2011-04-16 14:14:11 EDT

... but yes... I agree that from the optimization/performance point of view we should not execute unnecessary code (update scrollbars) when widget (Table,Tree and maybe other) is in dispose.

Comment 13 Rüdiger Herrmann 2011-04-16 14:36:26 EDT

As described in comment #11, the issue described here is not a valid use case.

I opened bug 343049 for the optimizations during dispose.