Bugzilla – Full Text Bug Listing
| Summary: | [Draw2d] FigureUtilities#setFont does not check for disposed Fonts | ||
|---|---|---|---|
| Product: | [RT] RAP | Reporter: | Jerome Sivadier <jerome.sivadier> |
| Component: | Incubator | Assignee: | Arnaud MERGEY <a_mergey> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | jerome.sivadier |
| Version: | 2.3 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| URL: | https://www.eclipse.org/forums/index.php/m/1699915/#msg_1699915 | ||
| Whiteboard: | |||
A proper fix is probably to remove static usage in FigureUtilities it should be fix now, please reopen if not (In reply to Arnaud MERGEY from comment #1) > A proper fix is probably to remove static usage in FigureUtilities > > it should be fix now, please reopen if not Still does not work with my test case (which I cannot provide, I am really sorry about that). In the situation I am facing, I have the following elements: - FontUtilities instance * appliedFont= "Font {Arial,7,BOLD}, id=444, disposed=true" * GC = a_gc * metrics = null - setFont parameter: "Font {Arial,7,BOLD}, id=490, disposed=false" Then, "f != null && f.equals(fu.appliedFont)" is true so the font is NOT replaced and I am facing a *DISPOSED* exception later on. Here is the relevant stack trace: Caused by: org.eclipse.swt.SWTException: Graphic is disposed at org.eclipse.swt.SWT.error(SWT.java:3658) at org.eclipse.swt.SWT.error(SWT.java:3581) at org.eclipse.swt.SWT.error(SWT.java:3552) at org.eclipse.swt.graphics.Font.getFontData(Font.java:155) at org.eclipse.swt.internal.graphics.FontUtil.getData(FontUtil.java:20) at org.eclipse.rap.rwt.internal.textsize.TextSizeUtil.lookup(TextSizeUtil.java:123) at org.eclipse.rap.rwt.internal.textsize.TextSizeUtil.determineTextSize(TextSizeUtil.java:97) at org.eclipse.rap.rwt.internal.textsize.TextSizeUtil.stringExtent(TextSizeUtil.java:42) at org.eclipse.swt.graphics.ControlGC.stringExtent(ControlGC.java:187) at org.eclipse.swt.graphics.GC.stringExtent(GC.java:227) at org.eclipse.draw2d.FigureUtilities.getStringDimension(FigureUtilities.java:136) at org.eclipse.draw2d.FigureUtilities.getStringExtents(FigureUtilities.java:165) at org.eclipse.draw2d.TextUtilities.getStringExtents(TextUtilities.java:45) Best regards, Jérôme Without a test case it will be hard to find what is going on. Regarding your stack trace, could you check your code in order to verify you do not keep any reference to a TextUtilities or a Font instance itself, that would be shared between multiple session threads ? Thanks for your reply. In my code I am always calling: - TextUtilities.INSTANCE().getStringExtents(String, Font) There are two calls which fails: one from "my" code where the Font is a new one, created for the calculation and then disposed; the other from the Label#calculateTextSize() code from draw2d. In both cases the problem is that when #setFont() is called, FigureUtilities.appliedFont is not null but *DISPOSED* and this "appliedFont" is the exact same (FontData speaking) as the new one. Again, I am sorry I cannot provide a test case (mainly because I am working on the porting of a big Graphiti-based application using GEF and Draw2d incubation projects on RAP [and it works!]) and I tried to use the suggested changes in a cloned version of org.eclipse.rap.draw2d#FigureUtilities and it works perfectly! The fix I made was just to remove static fields from FigureUtilities, so Font instances managed by FigureUtilities are not supposed to be shared between multiple UI Session thread (for example to avoid a UI Session to dispose the Font consumed by other UI Session in this case). If the issue is still there, a Font previously set in FigureUtilities is probably disposed in the same UI Session thread that created it. You need to track where or by who this Font is disposed, so unless the Font is disposed by some RAP code, if your app is a single sourced app, the issue is supposed to be consistent in RCP and RAP. To keep consistent behavior, between RCP and RAP, and in order to implement proper fix, we need to understand why and where the Font set in FigureUtilities.setFont is disposed. Hope I was clear enough So I digged down the problem and it comes from the "equals" implementation between Fonts actually.
In RAP the test case to reproduce the bug should be the following (here in pseudo code):
int i=0;
while(i++ < 3) {
Font f = new org.eclipse.swt.graphics.Font(null, new FontData[] { new FontData("Arial", 7, SWT.NORMAL) });
TextUtilities.INSTANCE().getStringExtents("Bla", f);
if (!f.isDisposed())
f.dispose()
}
I didn't have time to try this code but I'm pretty sure it crashes on RAP because the Font is disposed.
The reason is the following: on both RCP and RAP, FigureUtilities#setFont() checks if the new font is equals as object to the old one, or if "equals" returns true. But:
- On RCP, "equals" returns this.handle == ((Font)object).handle
- On RAP, "equals" returns font.internalFontData.equals( internalFontData ) which means internalFontData.name.equals( toCompare.internalFontData.name ) && internalFontData.height == toCompare.internalFontData.height && internalFontData.style == toCompare.internalFontData.style;
Indeed, in RAP NO CHECK is done regarding *DISPOSED* fonts and fonts are only compared regarding their FontData whereas in RCP the handles are different when two Fonts have the same FontData but are not the same (object speaking)...
I would suggest either to look for disposed Fonts or to modify the "Font.equals" method to check for disposed Fonts!
Regards,
Jérôme
|
Dear RAP developers, I've just discovered a strange behavior on the component "org.eclipse.rap.draw2d", with the class "FigureUtilities". Actually this class stores internally a reference to the last Font set (@see #setFont), and when a new Font is given, this last method compares the old Font and the new one to check if it needs to update the Font reference. Here is a copy of the method: protected static void setFont(Font f) { if (appliedFont == f || (f != null && f.equals(appliedFont))) return; getGC().setFont(f); appliedFont = f; metrics = null; } On the first line, we have a check to look if the new Font is not similar to the old one, and if so it does not swap the Fonts. The problem is that "f.equals(appliedFont)" DOES NOT check for DISPOSED Fonts indeed a reference to an old Font can be kept and this will provoke errors later during the execution; I've faced such errors in one of my projects. I propose the following correction, which has solved the issue on my project: protected static void setFont(Font f) { if (appliedFont == f || (f != null && f.equals(appliedFont) && !appliedFont.isDisposed())) return; getGC().setFont(f); appliedFont = f; metrics = null; } Best regards, Jerome