Posted by pookzilla on January 11, 2008
While trying to clean up and refactor the Equinox XSLT transform code (a moderately complex Equinox framework extension broken out among several bundles) that I wrote last year I find myself in one of three situations:
- I can have code that doesn’t compile without discouraged access warnings but works just fine in a running instance.
- I can have code that compiles cleanly but wont actually work in a running instance by virtue of how Equinox loads framework extensions.
- I can have code that compiles cleanly, runs fine, but is held together by spite, twine, and reflection.
Sigh.
Some days you’re the windshield, other days you’re the bug. 
Posted by pookzilla on December 18, 2007
There is another variant of the workbench startup problem I talked about previously that is outlined in bug 195664. In a nutshell, the problem is this.
In your editor (or view) part init method you need to pop up a progress dialog for some reason. When that kicks off, it uses a sync exec to do some work within ModalContext. After the workbench has started, this isn’t an issue, but if you hit this on startup there’ll be issues. The workbench is in a state where non-startup (a)sync execs are batched up for execution at a later time, after initialization is complete. Since we’re invoking one of these guys from the UI thread and waiting for it to complete we’re going to deadlock. Boourns.
So, what to do? Well, to start with, you probably don’t want to do any operation long enough to need a progress bar in the init method. Instead of performing the operation at that time instead defer the operation until after the editor is initialized. To do this you can use UI jobs. For instance, imagine an editor that prompts for a log-in before showing contents and then shows progress while the log-in is verified. The code could look something like the following:
[code lang="java"]
package interactiveEditor;
// snip imports
public class InteractiveTextEditorWithLogin extends TextEditor {
private StackLayout layout = new StackLayout();
private Composite composite;
public void init(final IEditorSite site, IEditorInput input)
throws PartInitException {
setSite(site);
setInput(input);
Job loginJob = new LoginJob("login", site);
loginJob.schedule();
}
public void createPartControl(Composite parent) {
composite = new Composite(parent, SWT.NONE);
composite.setLayout(layout);
Label label = new Label(composite, SWT.NONE);
label.setText("Please log in.");
layout.topControl = label;
}
protected void createRealContents() {
Composite realComposite = new Composite(composite, SWT.NONE);
realComposite.setLayout(new FillLayout());
super.createPartControl(realComposite);
layout.topControl = realComposite;
composite.layout(true);
}
private final class LoginJob extends UIJob {
private final IEditorSite site;
private LoginJob(String name, IEditorSite site) {
super(name);
this.site = site;
}
public IStatus runInUIThread(IProgressMonitor monitor) {
Dialog dialog = new LoginDialog(site.getWorkbenchWindow()
.getShell(), site);
dialog.open();
return Status.OK_STATUS;
}
}
private final class LoginDialog extends Dialog {
private final class LoginRunnable implements IRunnableWithProgress {
public void run(IProgressMonitor monitor)
throws InvocationTargetException,
InterruptedException {
monitor.beginTask("Logging in", 10);
for (int i = 0; i < 10; i++) {
monitor.worked(1);
Thread.sleep(500);
}
}
}
private final IEditorSite site;
private LoginDialog(Shell parentShell, IEditorSite site) {
super(parentShell);
this.site = site;
}
protected Control createDialogArea(Composite parent) {
Composite composite = (Composite) super.createDialogArea(parent);
Label nameLabel = new Label(composite, SWT.NONE);
nameLabel.setText("Name:");
new Text(composite, SWT.SINGLE);
Label passwordLabel = new Label(composite, SWT.NONE);
passwordLabel.setText("Password:");
new Text(composite, SWT.PASSWORD);
return composite;
}
protected void createButtonsForButtonBar(Composite parent) {
createButton(parent, 1, "Login", true);
}
protected void buttonPressed(int buttonId) {
close();
}
public boolean close() {
boolean code = super.close();
ProgressMonitorDialog pDialog = new ProgressMonitorDialog(site
.getWorkbenchWindow().getShell());
try {
pDialog.run(false, false, new LoginRunnable());
} catch (InvocationTargetException e) {
e.printStackTrace();
} catch (InterruptedException e) {
e.printStackTrace();
}
createRealContents();
return code;
}
}
}
[/code]
Instead of opening the login dialog (and subsequent progress dialog) directly in init we instead spawn a job that will open the the dialogs when executed. The initial contents of the editor is a simple label and only after the heavy lifting has been performed by the job are the real controls for the editor created and populated. This technique will work both when the editor is opened in a running Eclipse instance and when it is restored on startup in subsequent Eclipse instances – the workbench is smart enough not to run UIJobs until everything is settled after a startup.
So, this code will work but it still isn’t necessarily a good idea. Opening a dialog as a consequence of opening an editor is bad form. What would happen if you had two of these editors visible on startup? You’d have two dialogs opened with no idea which editor they belonged to. Instead, you might want to try the following code instead:
[code lang="java"]
package interactiveEditor;
// snip imports
public class InteractiveTextEditorWithLoginNoJobs extends TextEditor {
private final class LoginRunnable implements IRunnableWithProgress {
public void run(IProgressMonitor monitor)
throws InvocationTargetException, InterruptedException {
monitor.beginTask("Logging in", 5);
for (int i = 0; i < 5; i++) {
monitor.worked(1);
Thread.sleep(250);
}
}
}
private StackLayout layout = new StackLayout();
private Composite composite;
@Override
public void init(final IEditorSite site, IEditorInput input)
throws PartInitException {
setSite(site);
setInput(input);
}
@Override
public void createPartControl(Composite parent) {
composite = new Composite(parent, SWT.NONE);
composite.setLayout(layout);
Composite loginComposite = new Composite(composite, SWT.NONE);
loginComposite.setLayout(new GridLayout(2, false));
Label label = new Label(loginComposite, SWT.NONE);
label.setText("Please log in.");
label.setLayoutData(new GridData(GridData.BEGINNING,
GridData.BEGINNING, true, false, 2, 1));
Label nameLabel = new Label(loginComposite, SWT.NONE);
nameLabel.setText("Name:");
nameLabel.setLayoutData(new GridData(GridData.BEGINNING,
GridData.BEGINNING, true, false, 1, 1));
Text nameText = new Text(loginComposite, SWT.SINGLE);
nameText.setLayoutData(new GridData(GridData.BEGINNING,
GridData.BEGINNING, true, false, 1, 1));
Label passwordLabel = new Label(loginComposite, SWT.NONE);
passwordLabel.setText("Password:");
passwordLabel.setLayoutData(new GridData(GridData.BEGINNING,
GridData.BEGINNING, true, false, 1, 1));
Text passwordText = new Text(loginComposite, SWT.PASSWORD);
passwordText.setLayoutData(new GridData(GridData.BEGINNING,
GridData.BEGINNING, true, false, 1, 1));
Button loginButton = new Button(loginComposite, SWT.PUSH);
loginButton.setText("Login");
loginButton.setLayoutData(new GridData(GridData.BEGINNING,
GridData.BEGINNING, true, false, 2, 1));
loginButton.addSelectionListener(new SelectionAdapter() {
public void widgetSelected(SelectionEvent e) {
ProgressMonitorDialog pDialog = new ProgressMonitorDialog(
getSite().getWorkbenchWindow().getShell());
try {
pDialog.run(false, false, new LoginRunnable());
} catch (InvocationTargetException ex) {
ex.printStackTrace();
} catch (InterruptedException ex) {
ex.printStackTrace();
}
createRealContents();
}
});
layout.topControl = loginComposite;
}
protected void createRealContents() {
Composite realComposite = new Composite(composite, SWT.NONE);
realComposite.setLayout(new FillLayout());
super.createPartControl(realComposite);
layout.topControl = realComposite;
composite.layout(true);
}
}
[/code]
This code will embed log in controls into the editor instead of opening a dialog to display them. This is a much less offensive solution, particularly in the restoration case. No one wants a dialog in the eye first thing in the morning after booting Eclipse!
Posted by pookzilla on December 5, 2007
People have been asking for the ability to filter the user interface in Eclipse based on some abstract notion of “roles” for many years now. We partially addressed this issue via the introduction of activities but this solution is problematic at best – activities were designed to be gradually revealed over time and they were never really intended for hard lockdown of the user interface. Any effort in this direction could be easily circumvented by a manual call to the Workbench activity manager, for instance. Nonetheless, people have been using it for this purpose simply because there has been no alternative.
There is a lot of discussion happening on bug 201052 (“[RCP] RCP should provide some role based access control to UI elements”) at the moment pertaining to this issue. There are numerous solutions sketched out as proofs of concept and any input into this would be greatly appreciated. This problem wasn’t on our radar at all for 3.4 and without community support it probably wont see the light of day. If this is a problem you’re interested in please take a look at the bug.
Posted by pookzilla on December 4, 2007
I’ve decided to move my Eclipse blog to Wordpress as well. Right now I’ve got all of my posts in a separate Eclipse category with all of the appropriate redirects in place. http://eclipse.pookzilla.net should now be redirecting to http://pookzilla.net/wp/topics/eclipse/. I THINK that part of the migration is done… if anyone starts noticing personal posts in my Eclipse blog please let me know.
Posted by pookzilla on November 20, 2007
So you’ve read Steve’s blog and you’re all excited to help out with the Cocoa port (as you should be). But then you grab the code and you’re worried. Just how do you do anything with this? This is all so new and rough. You’re plodding along, kicking the tires, and suddenly you’re assaulted by a segfault in the native library. Your pant burst into flame, your dog gives birth to kittens, and you start singing soprano. Where do you start? How do you track this issue down?
There are going to be crashes. When you come across one that smells like a segfault here’s a simple (if somewhat laborious) method to track it down.
- open NSObject and put the following line at the beginning of release():
System.out.println("Release: " + toString());
- launch and make the crash happen
- In your Console view you should see reams and reams of lines like this:
...
Release: org.eclipse.swt.internal.cocoa.NSBezierPath@dd4cd3
Release: org.eclipse.swt.internal.cocoa.NSAttributedString@deb323
Release: org.eclipse.swt.internal.cocoa.NSBezierPath@c2ee77
- Starting from the bottom up locate each subclass of NSObject (such as NSBezierPath) and either override it’s
release() method such that it’s empty or alter the existing release() method so that it does nothing.
- repeat steps 2-4 with each successive subclass you encounter until the crash stops happening. Something pertaining to the last class you changed is probably your culpret.
- restore the
release() method on the subclass to its original form so that the crash happens again. Remove the System.out.println as well if you no longer feel it’s helpful to you.
- find all callers to
release() on the subclass.
- from here it gets fuzzy. You’ll need to start commenting out calls to
release() and see if any particular disposal of the object is responsible. Perhaps release() is called but the object is still held in SWT and later used for another purpose. If commenting out any particular release() call doesn’t stop the crash from happening you start looking at how the object is being retained. If we’re using an object from another structure are we calling retain() or copy() on it? We should never be release()-ing anything we haven’t previously retain()‘d (explicitly or implicitly)
Here’s how I applied this technique to find the crash Steve alluded to that was preventing Eclipse from coming up. I added my System.out statement to NSObject.release() and started Eclipse. It crashed and the last thing to be released was NSAutoReleasePool. I added an empty release() method to this class and tried to launch again. This time it crashed on NSBezierPath. I added a no-op release() to NSBezierPath and tried again. No crash! I then reverted NSAutoReleasePool and turned my attention towards callers to NSBezierPath.release().
I found two of interest in GC and another in Path. I decided to look at GC first because it seemed more interesting than Path. I commented out one of the releases in GC.setClipping() and launched only to find that the crash persisted. I then commented out the one in GC.dispose() and still the crash persisted. This one was going to be a bit more difficult than a simple disposal. I turned my attention towards all the places we set the data.clipPath member (the member that contained a reference to an NSBezierPath). I discovered two places where we set the value of data.clipPath: we null it out in GC.dispose() after calling release() on it and we
both null it out and assign it in setClipping. I decide to look in setClipping() first because the dispose() looks innocuous enough.
In setClipping we’re releasing the previous value of data.clipPath, if any. We then go and assign the supplied NSBezierPath to the data.clipPath member. Hmm. Interesting. We’re releasing an object we haven’t retained in this method. We must be retaining it elsewhere, right? Let’s look. There are four callers to the setClipping() method that takes an NSBezierPath. The first, taking four integer arguments, constructs and retains a new NSBezierPath so that isn’t our culprit. The second, taking a Path, creates a copy() of the path NSBezierPath instance (which is implicitly retain()‘d). This one is probably good too. Next up at bat is setClipping that takes a Region as input. This one looks suspicious! Here we’re passing in the NSBezierPath of the Region object without retaining it. Decrementing the reference count of an object you haven’t previously increased the reference for is a surefire way of causing grief. If we simply add a retain() or copy() call to the NSBezierPath object we’re getting from Region.getPath() the crash should go away. And lo it does.
This virgin Cocoa port represents an incredibly awesome opportunity for the community to shine. People have been crying for a Cocoa port for years now and now there’s an opportunity for industrious contributors to grab the bull by the horns and make it happen. The rewards (in terms of good will alone) are enormous. The SWT team has done a remarkable job bootstrapping the effort to get this port underway and it would be a real shame if the community didn’t jump on board and make it happen. Long term we absolutely NEED Cocoa for continued existence on the Mac – Carbons days are numbered. If you’re an Eclipse user on the Mac you owe it to yourself and the community to participate in this as best you can. If you don’t feel you’re able to submit patches then well-investigated bug reports will do. This is a huge amount of work and every little bit helps!
Posted by pookzilla on October 26, 2007
I don’t think I’ve ever tried to mark a bug as “gay” before but Firefox really wants me to for some reason. Truth be told the bug is rather fabulous – a greatbug in fact.
Posted by pookzilla on August 29, 2007
Thanks to our men in Zürich we now have a reusable control that can be embedded into Wizard pages/arbitrary dialogs that allows addition of resources or generic IAdaptables to a collection of working working sets. This means that we can FINALLY put an end to the deluge of bugs that (correctly) complain about how new resources don’t automatically appear in working sets. There is still a bit of behaviour that may need tweaking with this implementation (adding to working sets as a default action for instance) but the API is in pretty stable shape and we would appreciate any feedback you could give.
The implementation for this feature can be found in org.eclipse.ui.dialogs.WorkingSetConfigurationBlock and org.eclipse.ui.dialogs.WorkingSetGroup. The first class is the meat of the API and provides a reusable control for selecting the working sets. The second class simply wraps the first in an SWT group control with a standardized label.
Here’s what the API looks like in our plain old generic New Project wizard:
Posted by pookzilla on August 16, 2007
Edit: as it happens Tom has already written about this very problem, although approached from the splash screen angle. His solution is equally valid but I will keep this post around simply because it more specially calls out the threading issue that needs to be addressed.
Eclipse 3.3 brought some pretty substantial changes to the Eclipse startup process, many of which were to accommodate having a splash screen implemented in SWT. The most fundamental of these changes is that the Workbench initialization procedure is done on a non-UI thread while the UI thread spins the event loop in order to paint the splash. The initialization process often posts runnables back to the UI thread periodically to do various initialization segments that touch the UI but otherwise it’s done on it’s own thread so that the splash remains responsive. We’ve taken steps to ensure that any client-implementable API is still invoked on the UI thread as well to ensure backwards compatibility. Having said that, there are some patterns (which were never properly API) that used to work which no longer do.
These problems are most often encountered by RCP developers who’re trying to affect the UI during startup via a background thread using asyncExec or syncExec. Previous to 3.3 any runnable posted in this way would not be run until the Workbench had finished initializing or the app spun the loop manually. Given that we’re spinning the event loop early for all clients now it was important that any runnable posted would still not be run until after the workbench had finished initializing – running them early could cause all sorts of grief for clients that were assuming that the Workbench had already been created and initialized. The side effect of this, however, is that RCP developers who were creating applications that were interactive during initialization (providing a log in window for instance) could no longer spin the event loop themselves and expect background runnables to run.
We plan on providing some kind of API in 3.4 to more easily address this (see bug 182176) but in the mean time take a look at the following WorkbenchWindowAdvisor implementation.
The pattern, in a nutshell, is as follows. The RCP app spawns a background thread to do some form of grunt work during initialization. The RCP app then spins the event loop, periodically checking to see of the background thread has finished by seeing if it has created a certain runnable. If it has, then the runnable is invoked on the UI thread and the RCP app stops spinning the loop.
Posted by pookzilla on August 15, 2007
We’ve finally reached bug 200,000. It took us almost four years to get to bug 100,000 and only just over two to get to 200,000. Here’s to hoping we’re not talking about bug 300,000 a year from now.
Posted by pookzilla on June 14, 2007
If there’s one thing that’ll motivate me to post it’s the need to defend myself.
Yes, we removed the default visibility of the menu and toolbar Working Set items. We realize for some people that this will be annoying (even worthy of a posting!) but the fact of the matter is we have too many menu and toolbar items. We’re awash in them in fact. Some might say there is a glut. They are in abundance. You get the picture. Making this situation worse is the tendency for us to feel that our own items are SO important that they must be on by default not only in our own perspectives but everyone else’s as well. Given that some portion of our user base does not and never will use Working Sets we though it responsible that we remove the default visibility from these items. Returning their visibility is a trivial matter – it’s one simple trip to the Customize Perspective dialog. If you are truly put out by this change far beyond the point of reason and sanity please feel free to the “Working Set Tweaklet Feature” found at the brand new Platform UI incubator update site. It adds the Working Set actions to all perspectives by default. Also, there are some other “tweaklets” there that may be of interest to you.
You are in a maze of twisty little passages, all alike.
