Use a Job as the Reconciler Thread for AbstractReconciler #3025

merks · 2025-06-05T13:02:13Z

Currently AbstractReconciler uses a raw thread for reconciling requests. This has the problem that if the job framework is suspended (e.g, during startup of the IDE) some code is still executed because the reconciler is called from the creation of the text viewer and starts immediately.

This replaces the bare thread with a job but retains current behaviour. Note that the job cannot be manually terminated, just as the thread continued to run indefinitely until explicitly terminated.

#3018

merks · 2025-06-05T13:04:59Z

@iloveeclipse

I was able to reproduce the problem you described and with this change the job continues to run which avoids the described problem. In addition, I see that the editor starts with error markers that are subsequently cleaned up...

eclipse-platform-bot · 2025-06-05T13:07:17Z

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From 2bc2545e0c9c3193ada723c3a4a764984ea6535d Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Fri, 6 Jun 2025 12:53:36 +0000
Subject: [PATCH] Version bump(s) for 4.37 stream


diff --git a/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF b/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF
index d1429ea746..a5507fd35c 100644
--- a/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jface.text
-Bundle-Version: 3.28.0.qualifier
+Bundle-Version: 3.28.100.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: 
-- 
2.49.0

Further information are available in Common Build Issues - Missing version increments.

github-actions · 2025-06-05T13:56:34Z

Test Results

1 824 files ±0 1 824 suites ±0 1h 37m 39s ⏱️ + 1m 59s
7 925 tests ±0 7 697 ✅ ±0 228 💤 ±0 0 ❌ ±0
23 862 runs ±0 23 114 ✅ ±0 748 💤 ±0 0 ❌ ±0

Results for commit 03da687. ± Comparison against base commit 7429002.

♻️ This comment has been updated with latest results.

laeubi

Thant for catching up with this, ignoring the cancel request seems like a reasonable approach here, maybe we even want to override cancel() of the job and make it a noop with a comment why it is done here that way.

merks · 2025-06-05T14:11:26Z

Thant for catching up with this, ignoring the cancel request seems like a reasonable approach here, maybe we even want to override cancel() of the job and make it a noop with a comment why it is done here that way.

For our safety and comfort, that's not possible. 😁

https://github.com/eclipse-platform/eclipse.platform/blob/c3dfa7bc07048f1db682284b32a4011290b81c55/runtime/bundles/org.eclipse.core.jobs/src/org/eclipse/core/runtime/jobs/Job.java#L289-L291

iloveeclipse

This looks better now, except two places.

I guess we might see some test hangs or slowdown because of that change for tests that open editors and do waitForJobs(), we will see...

Ideally if we submit that it would be after we have at least one "green" build in SDK.

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java

merks · 2025-06-06T11:07:12Z

@iloveeclipse

All the failures seem related to an unrelated API filter problem:

iloveeclipse · 2025-06-06T11:20:36Z

@iloveeclipse

All the failures seem related to an unrelated API filter problem:

@raghucssit : I guess it's same issue you mentioned?

merks · 2025-06-06T11:23:42Z

@iloveeclipse

I'll look at fixing that API issue too, but it's hard to juggle back and forth. Also Ant current has errors in my workspace and Maven Publishing stopped working. Too many fires to fight. 😱

So if you're okay with the current content of this PR, maybe we could clear it out of the way?

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/junk

Currently AbstractReconciler uses a raw thread for reconciling requests. This has the problem that if the job framework is suspended (e.g, during startup of the IDE) some code is still executed because the reconciler is called from the creation of the text viewer and starts immediately. This replaces the bare thread with a job but retains current behaviour. Note that the job cannot be manually terminated, just as the thread continued to run indefinitely until explicitly terminated. eclipse-platform#3018

merks · 2025-06-06T12:49:41Z

Hopefully it builds this time with no failures!

merks · 2025-06-06T13:40:38Z

@iloveeclipse

We even have a green build now. 😁

iloveeclipse

Ed, please check SDK test results on the next SDK build. I fear there could be some hangs.

merks · 2025-06-06T14:04:48Z

I will look at the builds tomorrow.

merks · 2025-06-06T14:07:26Z

FYI, I started this build:

https://ci.eclipse.org/releng/job/Builds/job/I-build-4.37/14/

- This ensures that the tests don't leak jobs that are only detected by a subsequent test. - Use editor.close(false) rather than editor.dispose() to clean up the thread/job properly. eclipse-platform#3025

- This ensures that the tests don't leak jobs that are only detected by a subsequent test. - Use editor.close(false) rather than editor.dispose() to clean up the thread/job properly. #3025

Not so nice workaround for eclipse-jdt#721. After eclipse-platform/eclipse.platform.ui#3025 every opened editor means there will be a reconciler job running, which is not what we do NOT want to wait for. AbstractReconciler.class is the family object we can check for, so add that family to the list of excluded jobs we don't care.

After eclipse-platform/eclipse.platform.ui#3025 every opened editor means there will be a reconciler job running, which is not what we do NOT want to wait for. AbstractReconciler.class is the family object we can check for, so add that family to the list of excluded jobs we don't care. Added getUsualJobFamiliesToIgnore() and changed waitForJobs() to use that by default if nothing else is given. Fixes eclipse-jdt#721

After eclipse-platform/eclipse.platform.ui#3025 every opened editor means there will be a reconciler job running, which is not what we do NOT want to wait for. AbstractReconciler.class is the family object we can check for, so add that family to the list of excluded jobs we don't care. Added getUsualJobFamiliesToIgnore() and changed waitForJobs() to use that by default if nothing else is given. Fixes #721

iloveeclipse · 2025-07-04T12:02:58Z

Probably related regression: eclipse-jdt/eclipse.jdt.ui#2323

After eclipse-platform/eclipse.platform.ui#3025 every opened editor means there will be a reconciler job running, which is not what we do NOT want to wait for. AbstractReconciler.class is the family object we can check for, so add that family to the list of excluded jobs we don't care. Added getUsualJobFamiliesToIgnore() and changed waitForJobs() to use that by default if nothing else is given. Fixes #721

merks mentioned this pull request Jun 5, 2025

Use a Job as the Reconciler Thread #3018

Closed

merks requested a review from iloveeclipse June 5, 2025 13:03

laeubi approved these changes Jun 5, 2025

View reviewed changes

iloveeclipse requested changes Jun 5, 2025

View reviewed changes

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java Show resolved Hide resolved

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/AbstractReconciler.java Outdated Show resolved Hide resolved

merks force-pushed the pr-reconciler-job branch from 7d2fe33 to 8db735c Compare June 6, 2025 09:44

merks requested a review from iloveeclipse June 6, 2025 09:44

merks mentioned this pull request Jun 6, 2025

remove unused filter for RESCALING_AT_RUNTIME #3027

Merged

iloveeclipse reviewed Jun 6, 2025

View reviewed changes

bundles/org.eclipse.jface.text/src/org/eclipse/jface/text/reconciler/junk Outdated Show resolved Hide resolved

merks force-pushed the pr-reconciler-job branch from bc70653 to 7f32329 Compare June 6, 2025 12:49

merks requested a review from iloveeclipse June 6, 2025 12:49

Version bump(s) for 4.37 stream

03da687

iloveeclipse approved these changes Jun 6, 2025

View reviewed changes

merks merged commit 9360b5c into eclipse-platform:master Jun 6, 2025
18 checks passed

merks deleted the pr-reconciler-job branch June 6, 2025 14:06

merks mentioned this pull request Jun 7, 2025

Use TestUtil.cleanUp across the tests #3029

Merged

iloveeclipse mentioned this pull request Jun 12, 2025

CI-build time exceeds the timeout eclipse-jdt/eclipse.jdt.debug#721

Closed

iloveeclipse mentioned this pull request Jun 12, 2025

Avoid waiting for reconciler job eclipse-jdt/eclipse.jdt.debug#722

Merged

iloveeclipse mentioned this pull request Jul 4, 2025

Freeze / slow mark occurrences on selection changes in Java editor eclipse-jdt/eclipse.jdt.ui#2323

Closed

iloveeclipse mentioned this pull request Jul 16, 2025

[4.35] On startup Java editor shows error annotations on valid code that don't always disappear eclipse-jdt/eclipse.jdt.ui#2069

Closed

Use a Job as the Reconciler Thread for AbstractReconciler #3025

Use a Job as the Reconciler Thread for AbstractReconciler #3025

Uh oh!

Conversation

merks commented Jun 5, 2025

Uh oh!

merks commented Jun 5, 2025

Uh oh!

eclipse-platform-bot commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

laeubi left a comment

Choose a reason for hiding this comment

Uh oh!

merks commented Jun 5, 2025

Uh oh!

iloveeclipse left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

merks commented Jun 6, 2025

Uh oh!

iloveeclipse commented Jun 6, 2025

Uh oh!

merks commented Jun 6, 2025

Uh oh!

Uh oh!

merks commented Jun 6, 2025

Uh oh!

merks commented Jun 6, 2025

Uh oh!

iloveeclipse left a comment

Choose a reason for hiding this comment

Uh oh!

merks commented Jun 6, 2025

Uh oh!

Uh oh!

merks commented Jun 6, 2025

Uh oh!

iloveeclipse commented Jul 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

eclipse-platform-bot commented Jun 5, 2025 •

edited

Loading

github-actions bot commented Jun 5, 2025 •

edited

Loading