Skip to content

Commit 1fc9f09

Browse files
tobias-melchertobiasmelcher
authored andcommitted
Fix partitioner leak and add null guard in
SourceViewer#computeStyleRanges()
1 parent 60614bc commit 1fc9f09

2 files changed

Lines changed: 52 additions & 10 deletions

File tree

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/source/SourceViewer.java

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1437,6 +1437,8 @@ private void uninstallTextViewer() {
14371437
*/
14381438
public List<StyleRange> computeStyleRanges(IDocument document, IRegion region) throws BadLocationException {
14391439
Assert.isTrue(Display.getCurrent() != null, "computeStyleRanges must be called from SWT UI thread"); //$NON-NLS-1$
1440+
IDocument originalDocument= getDocument();
1441+
Assert.isNotNull(originalDocument, "viewer must have a document before calling computeStyleRanges"); //$NON-NLS-1$
14401442
String partition= IDocumentExtension3.DEFAULT_PARTITIONING;
14411443
IPresentationReconciler reconciler= fPresentationReconciler;
14421444
if (reconciler instanceof IPresentationReconcilerExtension ext) {
@@ -1445,21 +1447,25 @@ public List<StyleRange> computeStyleRanges(IDocument document, IRegion region) t
14451447
partition= extPartition;
14461448
}
14471449
}
1448-
IDocument originalDocument= getDocument();
14491450
IDocumentPartitioner originalDocumentPartitioner= null;
1450-
if (document instanceof IDocumentExtension3
1451+
IDocumentExtension3 documentExt= null;
1452+
if (document instanceof IDocumentExtension3 docExt
14511453
&& originalDocument instanceof IDocumentExtension3 originalExt) {
1454+
documentExt= docExt;
14521455
originalDocumentPartitioner= originalExt.getDocumentPartitioner(partition);
14531456
}
1457+
IDocumentPartitioner externalDocPartitioner= null;
14541458
try {
1455-
if (originalDocumentPartitioner != null) {
1459+
if (originalDocumentPartitioner != null && documentExt != null) {
14561460
// Temporarily reconnect the partitioner to the external document so that
14571461
// presentation repairers compute highlighting against the right content.
1458-
// The finally block always restores it to the original document.
1462+
// The finally block always restores both documents to their original state.
1463+
externalDocPartitioner= documentExt.getDocumentPartitioner(partition);
14591464
originalDocumentPartitioner.disconnect();
14601465
originalDocumentPartitioner.connect(document);
1461-
((IDocumentExtension3) document).setDocumentPartitioner(partition, originalDocumentPartitioner);
1466+
documentExt.setDocumentPartitioner(partition, originalDocumentPartitioner);
14621467
} else {
1468+
externalDocPartitioner= document.getDocumentPartitioner();
14631469
document.setDocumentPartitioner(originalDocument.getDocumentPartitioner());
14641470
}
14651471
TextPresentation presentation= new TextPresentation(region, 1000);
@@ -1480,8 +1486,12 @@ public List<StyleRange> computeStyleRanges(IDocument document, IRegion region) t
14801486
}
14811487
return result;
14821488
} finally {
1483-
if (originalDocumentPartitioner != null) {
1489+
if (originalDocumentPartitioner != null && documentExt != null) {
1490+
originalDocumentPartitioner.disconnect();
14841491
originalDocumentPartitioner.connect(originalDocument);
1492+
documentExt.setDocumentPartitioner(partition, externalDocPartitioner);
1493+
} else {
1494+
document.setDocumentPartitioner(externalDocPartitioner);
14851495
}
14861496
}
14871497
}

tests/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/source/SourceViewerComputeStyleRangesTest.java

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,15 +129,20 @@ public void testOriginalDocumentNotAffected() throws Exception {
129129
SourceViewer viewer= createConfiguredViewer();
130130
String originalContent= "original content";
131131
Document originalDoc= new Document(originalContent);
132+
IDocumentPartitioner originalPartitioner= setupPartitioning(originalDoc);
133+
assertNotNull(originalPartitioner);
132134
viewer.setDocument(originalDoc);
133-
IDocumentPartitioner originalPartitioner= originalDoc.getDocumentPartitioner();
134135

135136
Document externalDoc= new Document("some 'highlighted' text");
137+
IDocumentPartitioner externalPartitioner= setupPartitioning(externalDoc);
138+
assertNotNull(externalPartitioner);
136139
viewer.computeStyleRanges(externalDoc, new Region(0, externalDoc.getLength()));
137140

138141
assertEquals(originalContent, originalDoc.get(), "Original document content must not change");
139142
assertEquals(originalPartitioner, originalDoc.getDocumentPartitioner(),
140143
"Original document partitioner must be restored");
144+
assertEquals(externalPartitioner, externalDoc.getDocumentPartitioner(),
145+
"External document partitioner must be restored");
141146
}
142147

143148
@Test
@@ -193,6 +198,10 @@ public void testExceptionSafetyPartitionerRestored() throws Exception {
193198

194199
Document externalDoc= new Document("short");
195200
setupNamedPartitioning(externalDoc);
201+
IDocumentPartitioner externalPartitioner= externalDoc
202+
.getDocumentPartitioner(NAMED_PARTITIONING);
203+
assertNotNull(externalPartitioner, "External document should have a named partitioner");
204+
196205
try {
197206
viewer.computeStyleRanges(externalDoc, new Region(0, 100));
198207
} catch (BadLocationException expected) {
@@ -203,17 +212,25 @@ public void testExceptionSafetyPartitionerRestored() throws Exception {
203212
IDocumentPartitioner restoredPartitioner= originalDoc
204213
.getDocumentPartitioner(NAMED_PARTITIONING);
205214
assertNotNull(restoredPartitioner, "Partitioner must be restored to original document after exception");
215+
assertEquals(originalPartitioner, restoredPartitioner);
216+
217+
IDocumentPartitioner restoredExternalPartitioner= externalDoc
218+
.getDocumentPartitioner(NAMED_PARTITIONING);
219+
assertNotNull(restoredExternalPartitioner, "External partitioner must be restored to original document after exception");
220+
assertEquals(externalPartitioner, restoredExternalPartitioner);
206221
}
207222

208223
@Test
209224
public void testNamedPartitioning() throws Exception {
210225
SourceViewer viewer= createConfiguredViewerWithNamedPartitioning();
211226
Document originalDoc= new Document("original 'content' here");
212-
setupNamedPartitioning(originalDoc);
227+
IDocumentPartitioner originalPartitioner= setupNamedPartitioning(originalDoc);
228+
assertNotNull(originalPartitioner);
213229
viewer.setDocument(originalDoc);
214230

215231
Document externalDoc= new Document("external 'styled' text");
216-
setupNamedPartitioning(externalDoc);
232+
IDocumentPartitioner externalPartitioner= setupNamedPartitioning(externalDoc);
233+
assertNotNull(externalPartitioner);
217234
List<StyleRange> styles= viewer.computeStyleRanges(externalDoc, new Region(0, externalDoc.getLength()));
218235

219236
assertNotNull(styles);
@@ -223,6 +240,12 @@ public void testNamedPartitioning() throws Exception {
223240
IDocumentPartitioner restoredPartitioner= originalDoc
224241
.getDocumentPartitioner(NAMED_PARTITIONING);
225242
assertNotNull(restoredPartitioner, "Partitioner must be restored to original document");
243+
assertEquals(originalPartitioner, restoredPartitioner);
244+
245+
IDocumentPartitioner restoredExternalPartitioner= externalDoc
246+
.getDocumentPartitioner(NAMED_PARTITIONING);
247+
assertNotNull(restoredExternalPartitioner, "External partitioner must be restored to external document");
248+
assertEquals(externalPartitioner, restoredExternalPartitioner);
226249
}
227250

228251
private SourceViewer createConfiguredViewer() {
@@ -266,10 +289,19 @@ private RuleBasedScanner createScanner() {
266289
return scanner;
267290
}
268291

269-
private void setupNamedPartitioning(Document document) {
292+
private IDocumentPartitioner setupNamedPartitioning(Document document) {
270293
IPartitionTokenScanner partitionScanner= new RuleBasedPartitionScanner();
271294
IDocumentPartitioner partitioner= new FastPartitioner(partitionScanner, new String[] {});
272295
document.setDocumentPartitioner(NAMED_PARTITIONING, partitioner);
273296
partitioner.connect(document);
297+
return partitioner;
298+
}
299+
300+
private IDocumentPartitioner setupPartitioning(Document document) {
301+
IPartitionTokenScanner partitionScanner= new RuleBasedPartitionScanner();
302+
IDocumentPartitioner partitioner= new FastPartitioner(partitionScanner, new String[] {});
303+
document.setDocumentPartitioner(partitioner);
304+
partitioner.connect(document);
305+
return partitioner;
274306
}
275307
}

0 commit comments

Comments
 (0)