Skip to content

Don't call pack() on not yet visible table on Linux#947

Closed
iloveeclipse wants to merge 2 commits intoeclipse-equinox:masterfrom
iloveeclipse:issue_946
Closed

Don't call pack() on not yet visible table on Linux#947
iloveeclipse wants to merge 2 commits intoeclipse-equinox:masterfrom
iloveeclipse:issue_946

Conversation

@iloveeclipse
Copy link
Copy Markdown
Member

Something goes wrong on SWT Column/Table GTK code if column is being packed on not yet shown table. So let pack() the column on a next call on Linux.

Fixes #946

Something goes wrong on SWT Column/Table GTK code if column is being
packed on not yet shown table. So let pack() the column on a next call
on Linux.

Fixes eclipse-equinox#946
@laeubi
Copy link
Copy Markdown
Member

laeubi commented Sep 17, 2025

Something goes wrong on SWT Column/Table GTK code if column is being packed on not yet shown table.

Should this not fixed at SWT instead on each call site of pack?

@eclipse-equinox-bot
Copy link
Copy Markdown
Contributor

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.equinox.p2.ui.sdk/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 dc36f2618c08d736175468ef83eaa7f1086c0f26 Mon Sep 17 00:00:00 2001
From: Eclipse Equinox Bot <equinox-bot@eclipse.org>
Date: Wed, 17 Sep 2025 14:59:29 +0000
Subject: [PATCH] Version bump(s) for 4.38 stream


diff --git a/bundles/org.eclipse.equinox.p2.ui.sdk/META-INF/MANIFEST.MF b/bundles/org.eclipse.equinox.p2.ui.sdk/META-INF/MANIFEST.MF
index 8a807ca97..9f157ef37 100644
--- a/bundles/org.eclipse.equinox.p2.ui.sdk/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.equinox.p2.ui.sdk/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %bundleName
 Bundle-SymbolicName: org.eclipse.equinox.p2.ui.sdk;singleton:=true
-Bundle-Version: 1.3.500.qualifier
+Bundle-Version: 1.3.600.qualifier
 Bundle-Activator: org.eclipse.equinox.internal.p2.ui.sdk.ProvSDKUIActivator
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
-- 
2.51.0

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

@iloveeclipse
Copy link
Copy Markdown
Member Author

Something goes wrong on SWT Column/Table GTK code if column is being packed on not yet shown table.

Should this not fixed at SWT instead on each call site of pack?

If one knows where and how to fix, for sure. I don't know.

Copy link
Copy Markdown
Member

@laeubi laeubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It don't seems suitable to add workarounds that complicate the code her at P2 and should better be reported / fixed at SWT.

JDT shows me 162 references to TableColumn#pack() where all of these has to be "fixed" this way then or are prone to the same error.

@merks
Copy link
Copy Markdown
Contributor

merks commented Sep 17, 2025

Yes, I'm not happy to see such changes in client code that worked well in the past. This doesn't feel right. How many other place are affected?

@iloveeclipse
Copy link
Copy Markdown
Member Author

@laeubi : Of course it would be better if we could investigate and fix every bug, but in the reality most of us have no resources to do so. I'm unable to spend time on investigating some probably weird SWT / GTK specific issue which from my experience is non trivial and at the end might even has no fix at all because it would require GTK fix and that will never happen.

I'm unaware about other issues in SDK that result in the StackOverflow. By blocking this trivial workaround you accepting that we will continue ship SDK with a known error and users will run into it.

If you have time to inestigate this issue and would commit to provide SWT or JFace fix for it for 4.38, it is OK to block.
If not, please consider to unblock merging of this PR.

@github-actions
Copy link
Copy Markdown

Test Results

  256 files  ±0    256 suites  ±0   32m 27s ⏱️ + 2m 9s
1 899 tests ±0  1 896 ✅ ±0  3 💤 ±0  0 ❌ ±0 
4 480 runs  ±0  4 474 ✅ ±0  6 💤 ±0  0 ❌ ±0 

Results for commit ab49549. ± Comparison against base commit d311315.

Copy link
Copy Markdown
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does line securedColumn.getColumn().pack() at line 621 have the same problem when turning to the other tab?

This feels like treating a symptom and hoping the disease has no impact elsewhere. ☹️

@iloveeclipse
Copy link
Copy Markdown
Member Author

Does line securedColumn.getColumn().pack() at line 621 have the same problem when turning to the other tab?

No.
What I see is also that column width reported at the pack() is zero and replacing pack() with setWidth(10) produces same StackOverflow. So pack() is not the main problem.

I guess that TableColumnLayout that uses ColumnWeightData in createColumn() method plays role here. Something like AbstractColumnLayout.resizeListener that is updating table size which again cause size change that again causes listeners to resize because table/column is not yet sized properly etc.

Calling pack() after the table is realized by GTK unbreaks the loop because table/columnsare visible and have the final sizes.

@iloveeclipse
Copy link
Copy Markdown
Member Author

Here is what I've seen in debugger that unbreaks the loop for me: eclipse-platform/eclipse.platform.ui#3283

I have NO IDEA if and how that will work in all other possible cases / platforms, so I do not plan to work on that.

@iloveeclipse
Copy link
Copy Markdown
Member Author

Closing in favour eclipse-platform/eclipse.platform.ui#3283

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Install/Update -> Trust" preference page can't be opened on Linux because of StackOverflowError

4 participants