Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix initialization of editor preferences when triggered in non-ui thread #43

Open
wants to merge 1 commit into
base: testing
Choose a base branch
from

Conversation

apaku
Copy link

@apaku apaku commented Sep 26, 2017

The preference-initialization code can be triggered from a background
thread, in particular from the build-workspace job. This happens when
enabling automatic refresh of workspace resources based on native hooks or
polling. In Eclipse 4.4 and later this triggers a workspace rebuild early
during the initialization sequence.

The workspace-build triggers reading the perl executable from the
preferences and that read triggers initialization of the default values.

Unfortunately the PreferenceConverter helper class has a static
initialization block accessing the display/ui thread for loading font
data (at least in Eclipse 4.4). Since the workspace-build-job is running
in a different thread this access fails leading to the PreferenceConverter
initialization to fail and hence the whole class not being loadable.

This breaks all kinds of plugins which use this class, ultimately leading
to Eclipse not starting up at all.

I chose to drop the use of this class and instead copy the 'logic' from
its setDefault overload for RGB values. The function uses another helper
class to do the actual conversion from RGB to a string value, so there's
almost no code-duplication created with this change.

I had tried to do a syncExec on the display instead, but that leads to
another problem, since it has to wait for the display thread to be
available. The display thread however is initializing the UI, in particular
also restoring editors, so that a restored perl editor wouldn't get access
to the proper default values for the colors. This leads to missing syntax
highlighting as well as a black side-bar.

Separating out the non-ui preferences into a separate non-ui plugin
would likely be an option too. However that requires quite a bit more
work than I'd want to invest for this single bug.

The preference-initialization code can be triggered from a background
thread, in particular from the build-workspace job. This happens when
enabling automatic refresh of workspace resources based on native hooks or
polling. In Eclipse 4.4 and later this triggers a workspace rebuild early
during the initialization sequence.

The workspace-build triggers reading the perl executable from the
preferences and that read triggers initialization of the default values.

Unfortunately the PreferenceConverter helper class has a static
initialization block accessing the display/ui thread for loading font
data (at least in Eclipse 4.4). Since the workspace-build-job is running
in a different thread this access fails leading to the PreferenceConverter
initialization to fail and hence the whole class not being loadable.

This breaks all kinds of plugins which use this class, ultimately leading
to Eclipse not starting up at all.

I chose to drop the use of this class and instead copy the 'logic' from
its setDefault overload for RGB values. The function uses another helper
class to do the actual conversion from RGB to a string value, so there's
almost no code-duplication created with this change.

I had tried to do a syncExec on the display instead, but that leads to
another problem, since it has to wait for the display thread to be
available. The display thread however is initializing the UI, in particular
also restoring editors, so that a restored perl editor wouldn't get access
to the proper default values for the colors. This leads to missing syntax
highlighting as well as a black side-bar.

Separating out the non-ui preferences into a separate non-ui plugin
would likely be an option too. However that requires quite a bit more
work than I'd want to invest for this single bug.
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.

1 participant