Skip to content

Commit dd8340b

Browse files
adangeladangel
committed
Merge pull request #148 from adangel:issue-148-ui-crash
Execute org.eclipse.swt.graphics.GC call in UI thread in ShapePainter #157
2 parents 00fb3c1 + 198d2e5 commit dd8340b

File tree

17 files changed

+94
-79
lines changed

17 files changed

+94
-79
lines changed

‎ReleaseNotes.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ This is a minor release.
1515

1616
### Fixed Issues
1717

18+
* [#148](https://github.com/pmd/pmd-eclipse-plugin/issues/148): Eclipse nearly-consistently crashes on startup when workspace contains PMD enabled projects
1819
* [#150](https://github.com/pmd/pmd-eclipse-plugin/issues/150): Error executing command ReviewCode: java.util.regex.PatternSyntaxException: Illegal/unsupported escape sequence near index
1920
* [#153](https://github.com/pmd/pmd-eclipse-plugin/issues/153): Not properly disposed SWT resource
2021

@@ -28,6 +29,7 @@ This is a minor release.
2829
* `net.sourceforge.pmd.eclipse.runtime.cmd.BaseVisitor.setUseTaskMarker(boolean)`
2930
* `net.sourceforge.pmd.eclipse.ui.preferences.br.FilterManager`
3031
* `net.sourceforge.pmd.eclipse.util.IOUtil`
32+
* `net.sourceforge.pmd.eclipse.ui.priority.PriorityDescriptor.refreshImages()`
3133
* All classes that couldn't be instantiated because they had a private constructor only
3234
are now also `final`.
3335

‎net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/plugin/PMDPlugin.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ public static IViewPart getView(String id) {
334334
* @param viewId id of the view
335335
*/
336336
public void refreshView(final String viewId) {
337-
Display.getCurrent().asyncExec(new Runnable() {
337+
Display.getDefault().asyncExec(new Runnable() {
338338
@Override
339339
public void run() {
340340
try {
@@ -447,7 +447,7 @@ public void showError(final String message, final Throwable t) {
447447
@Override
448448
public void run() {
449449
String errTitle = getStringTable().getString(StringKeys.ERROR_TITLE);
450-
MessageDialog.openError(Display.getCurrent().getActiveShell(), errTitle,
450+
MessageDialog.openError(Display.getDefault().getActiveShell(), errTitle,
451451
message + "\n" + String.valueOf(t));
452452
}
453453
});
@@ -462,7 +462,7 @@ public void showUserError(final String message) {
462462
@Override
463463
public void run() {
464464
String errTitle = getStringTable().getString(StringKeys.ERROR_TITLE);
465-
MessageDialog.openError(Display.getCurrent().getActiveShell(), errTitle, message);
465+
MessageDialog.openError(Display.getDefault().getActiveShell(), errTitle, message);
466466
}
467467
});
468468
}

‎net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/PageBuilder.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
*/
3838
public class PageBuilder {
3939
private static final char CR = '\n';
40-
private static final Color BACKGROUND = Display.getCurrent().getSystemColor(SWT.COLOR_WHITE);
40+
private static final Color BACKGROUND = Display.getDefault().getSystemColor(SWT.COLOR_WHITE);
4141

4242
private static final Comparator<StyleRange> STYLE_COMPARATOR = new Comparator<StyleRange>() {
4343
@Override
@@ -62,7 +62,7 @@ public PageBuilder(int textIndent, int headingColorIndex, FontBuilder codeFontBu
6262
buffer = new StringBuilder(500);
6363
indentDepth = textIndent;
6464

65-
Display display = Display.getCurrent();
65+
Display display = Display.getDefault();
6666
headingColor = display.getSystemColor(headingColorIndex);
6767
codeStyle = codeFontBuilder.style(display);
6868

‎net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/ShapePainter.java‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
package net.sourceforge.pmd.eclipse.ui;
66

7+
import java.util.concurrent.atomic.AtomicReference;
8+
79
import org.eclipse.swt.graphics.GC;
810
import org.eclipse.swt.graphics.Image;
911
import org.eclipse.swt.graphics.ImageData;
@@ -28,29 +30,38 @@ private ShapePainter() {
2830
* drawn within. It will return cached images to avoid creating new images
2931
* (and using the limited GDI handles under Windows).
3032
*/
31-
public static Image newDrawnImage(Display display, int width, int height, Shape shape, RGB transparentColour,
32-
RGB fillColour) {
33-
Image image = new Image(display, width, height);
34-
GC gc = new GC(image);
33+
public static Image newDrawnImage(final Display displayNotUsed, final int width, final int height, final Shape shape,
34+
final RGB transparentColour, final RGB fillColour) {
35+
36+
final AtomicReference<Image> result = new AtomicReference<>();
37+
Display.getDefault().syncExec(new Runnable() {
38+
@Override
39+
public void run() {
40+
Display display = Display.getDefault();
41+
Image image = new Image(display, width, height);
42+
GC gc = new GC(image);
3543

36-
gc.setBackground(PMDPlugin.getDefault().colorFor(transparentColour));
37-
gc.fillRectangle(0, 0, width, height);
44+
gc.setBackground(PMDPlugin.getDefault().colorFor(transparentColour));
45+
gc.fillRectangle(0, 0, width, height);
3846

39-
gc.setForeground(PMDPlugin.getDefault().colorFor(RGB_BLACK));
40-
gc.setBackground(PMDPlugin.getDefault().colorFor(fillColour));
47+
gc.setForeground(PMDPlugin.getDefault().colorFor(RGB_BLACK));
48+
gc.setBackground(PMDPlugin.getDefault().colorFor(fillColour));
4149

42-
drawShape(width - 1, height - 1, shape, gc, 0, 0, null);
50+
drawShape(width - 1, height - 1, shape, gc, 0, 0, null);
4351

44-
ImageData data = image.getImageData();
45-
int clrIndex = data.palette.getPixel(transparentColour);
46-
data.transparentPixel = clrIndex;
52+
ImageData data = image.getImageData();
53+
int clrIndex = data.palette.getPixel(transparentColour);
54+
data.transparentPixel = clrIndex;
4755

48-
Image newImage = new Image(display, data);
49-
image.dispose();
56+
Image newImage = new Image(display, data);
57+
image.dispose();
5058

51-
gc.dispose();
59+
gc.dispose();
5260

53-
return newImage;
61+
result.set(newImage);
62+
}
63+
});
64+
return result.get();
5465
}
5566

5667
@Deprecated

‎net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/ShapePicker.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ public void focusLost(FocusEvent e) {
118118
private Color colourFor(int itemIndex) {
119119
ShapeDescriptor desc = shapeDescriptorsByItem.get(items[itemIndex]);
120120
if (desc == null) {
121-
return Display.getCurrent().getSystemColor(SWT.COLOR_BLACK);
121+
return Display.getDefault().getSystemColor(SWT.COLOR_BLACK);
122122
}
123123

124124
RGB rgb = desc.rgbColor;

‎net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/actions/ClearReviewsAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public void init(IViewPart view) {
5757
@Override
5858
public void run(IAction action) {
5959
LOG.info("Remove violation reviews requested.");
60-
ProgressMonitorDialog monitorDialog = new ProgressMonitorDialog(Display.getCurrent().getActiveShell());
60+
ProgressMonitorDialog monitorDialog = new ProgressMonitorDialog(Display.getDefault().getActiveShell());
6161
try {
6262
monitorDialog.run(false, false, new IRunnableWithProgress() {
6363
@Override

‎net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/editors/StyleExtractor.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,12 @@ public class StyleExtractor {
2424
private SyntaxData syntaxData;
2525
private List<int[]> commentOffsets;
2626

27-
private static final Color COMMENT_COLOR = Display.getCurrent().getSystemColor(SWT.COLOR_DARK_GREEN);
28-
private static final Color REFERENCED_VAR_COLOR = Display.getCurrent().getSystemColor(SWT.COLOR_DARK_GREEN);
29-
private static final Color COMMENT_BACKGROUND = Display.getCurrent().getSystemColor(SWT.COLOR_WHITE);
30-
private static final Color PUNCTUATION_COLOR = Display.getCurrent().getSystemColor(SWT.COLOR_BLACK);
31-
private static final Color KEYWORD_COLOR = Display.getCurrent().getSystemColor(SWT.COLOR_DARK_MAGENTA);
32-
private static final Color STRING_COLOR = Display.getCurrent().getSystemColor(SWT.COLOR_BLUE);
27+
private static final Color COMMENT_COLOR = Display.getDefault().getSystemColor(SWT.COLOR_DARK_GREEN);
28+
private static final Color REFERENCED_VAR_COLOR = Display.getDefault().getSystemColor(SWT.COLOR_DARK_GREEN);
29+
private static final Color COMMENT_BACKGROUND = Display.getDefault().getSystemColor(SWT.COLOR_WHITE);
30+
private static final Color PUNCTUATION_COLOR = Display.getDefault().getSystemColor(SWT.COLOR_BLACK);
31+
private static final Color KEYWORD_COLOR = Display.getDefault().getSystemColor(SWT.COLOR_DARK_MAGENTA);
32+
private static final Color STRING_COLOR = Display.getDefault().getSystemColor(SWT.COLOR_BLUE);
3333

3434
public StyleExtractor(SyntaxData theSyntaxData) {
3535
syntaxData = theSyntaxData;

‎net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/preferences/br/AbstractTreeTableManager.java‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.util.List;
1010
import java.util.Map;
1111
import java.util.Objects;
12+
import java.util.concurrent.atomic.AtomicReference;
1213

1314
import org.eclipse.jface.viewers.CheckboxTreeViewer;
1415
import org.eclipse.jface.viewers.ISelectionChangedListener;
@@ -129,16 +130,21 @@ protected Map<Integer, List<Listener>> paintListeners() {
129130
}
130131

131132
protected void createCheckBoxColumn(Tree tree) {
132-
133133
TreeColumn tc = new TreeColumn(tree, 0);
134134

135-
Image image = new Image(Display.getCurrent(), 15, 15);
136-
GC gc = new GC(image);
137-
Point textExtent = gc.textExtent("m");
138-
gc.dispose();
139-
image.dispose();
135+
final AtomicReference<Point> textExtent = new AtomicReference<>();
136+
Display.getDefault().syncExec(new Runnable() {
137+
@Override
138+
public void run() {
139+
Image image = new Image(Display.getDefault(), 15, 15);
140+
GC gc = new GC(image);
141+
textExtent.set(gc.textExtent("m"));
142+
gc.dispose();
143+
image.dispose();
144+
}
145+
});
140146

141-
tc.setWidth(textExtent.x * 4);
147+
tc.setWidth(textExtent.get().x * 4);
142148

143149
tc.setResizable(true);
144150
tc.pack();

‎net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/preferences/editors/AbstractEditorFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ protected AbstractEditorFactory() {
3232
// private static ColourManager colourManager() {
3333
//
3434
// if (colourManager != null) return colourManager;
35-
// colourManager = ColourManager.managerFor(Display.getCurrent());
35+
// colourManager = ColourManager.managerFor(Display.getDefault());
3636
// return colourManager;
3737
// }
3838

‎net.sourceforge.pmd.eclipse.plugin/src/main/java/net/sourceforge/pmd/eclipse/ui/priority/PriorityDescriptor.java‎

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
package net.sourceforge.pmd.eclipse.ui.priority;
66

77
import java.util.EnumSet;
8-
import java.util.HashMap;
9-
import java.util.Map;
8+
import java.util.HashSet;
9+
import java.util.Set;
10+
import java.util.concurrent.ConcurrentHashMap;
11+
import java.util.concurrent.ConcurrentMap;
1012

1113
import org.apache.commons.lang3.StringUtils;
1214
import org.eclipse.jface.resource.ImageDescriptor;
@@ -44,10 +46,6 @@ public PriorityDescriptor(RulePriority thePriority, String theLabelKey, String t
4446
this(thePriority, theLabelKey, theFilterTextKey, theIconId, new ShapeDescriptor(theShape, theColor, theSize));
4547
}
4648

47-
private PriorityDescriptor(RulePriority thePriority) {
48-
priority = thePriority;
49-
}
50-
5149
public PriorityDescriptor(RulePriority thePriority, String theLabelKey, String theFilterTextKey, String theIconId,
5250
ShapeDescriptor theShape) {
5351
priority = thePriority;
@@ -166,6 +164,7 @@ public PriorityDescriptor clone() {
166164
copy.filterText = filterText;
167165
copy.iconId = iconId;
168166
copy.shape = shape.clone();
167+
copy.cachedImages = new ConcurrentHashMap<>(3);
169168
return copy;
170169
} catch (CloneNotSupportedException e) {
171170
throw new RuntimeException(e);
@@ -185,7 +184,7 @@ public Image getImage(Display display) {
185184
*/
186185
@Deprecated
187186
public Image getImage(Display display, int maxDimension) {
188-
return ShapePainter.newDrawnImage(display, Math.min(shape.size, maxDimension),
187+
return ShapePainter.newDrawnImage(null, Math.min(shape.size, maxDimension),
189188
Math.min(shape.size, maxDimension), shape.shape, PROTO_TRANSPARENT_COLOR, shape.rgbColor // fillColour
190189
);
191190
}
@@ -203,7 +202,7 @@ public String toString() {
203202
return sb.toString();
204203
}
205204

206-
private Map<Integer, Image> cachedImages = new HashMap<>(3);
205+
private ConcurrentMap<Integer, Image> cachedImages = new ConcurrentHashMap<>(3);
207206
private static final int ANNOTATION_IMAGE_DIMENSION = 9;
208207

209208
public Image getAnnotationImage() {
@@ -216,7 +215,7 @@ public ImageDescriptor getAnnotationImageDescriptor() {
216215

217216
private Image createImage(final int size) {
218217
if (iconId == null || iconId.isEmpty() || "null".equals(iconId)) {
219-
return ShapePainter.newDrawnImage(Display.getCurrent(), size, size, shape.shape, PROTO_TRANSPARENT_COLOR,
218+
return ShapePainter.newDrawnImage(null, size, size, shape.shape, PROTO_TRANSPARENT_COLOR,
220219
shape.rgbColor // fillColour
221220
);
222221
} else {
@@ -239,8 +238,16 @@ private Image createImage(final int size) {
239238
public Image getImage(int size) {
240239
Image cached = cachedImages.get(size);
241240
if (cached == null) {
242-
cached = createImage(size);
243-
cachedImages.put(size, cached);
241+
Image newImage = createImage(size);
242+
cached = cachedImages.putIfAbsent(size, newImage);
243+
if (cached == null) {
244+
cached = newImage;
245+
} else {
246+
// two threads might have created the same image, only one is needed
247+
// use the already cached image and dispose the new image, that was
248+
// unnecessarily created
249+
newImage.dispose();
250+
}
244251
}
245252
return cached;
246253
}
@@ -259,24 +266,20 @@ public Image createImage() {
259266
}
260267

261268
public void dispose() {
262-
for (Image image : cachedImages.values()) {
269+
Set<Image> copy = new HashSet<>(cachedImages.values());
270+
cachedImages.clear();
271+
272+
for (Image image : copy) {
263273
image.dispose();
264274
}
265-
cachedImages.clear();
266275
}
267276

268277
/**
269278
* Eagerly create the images.
279+
* @deprecated not used anymore, it does nothing anymore. The images are created on demand now.
270280
*/
281+
@Deprecated
271282
public void refreshImages() {
272-
dispose();
273-
Image image = getAnnotationImage();
274-
if (image == null) {
275-
throw new RuntimeException("Could not create annotation image");
276-
}
277-
image = getImage(16);
278-
if (image == null) {
279-
throw new RuntimeException("Could not create marker image");
280-
}
283+
// does nothing
281284
}
282285
}

0 commit comments

Comments
 (0)