No Business Naming Things

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:

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;
                }
        }
}

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:

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);
        }

}

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!

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.

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.

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.

1. open NSObject and put the following line at the beginning of release():
   System.out.println(”Release: ” + toString());
2. launch and make the crash happen
3. 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
4. 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.

5. 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.
6. 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.

7. find all callers to
   release()

   on the subclass.

8. 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

and started Eclipse. It crashed and the last thing to be released was

. I added an empty

method to this class and tried to launch again. This time it crashed on

. I added a no-op

to

and tried again. No crash! I then reverted

and turned my attention towards callers to

.

I found two of interest in

and another in

. I decided to look at

first because it seemed more interesting than

. I commented out one of the releases in

and launched only to find that the crash persisted. I then commented out the one in

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

member (the member that contained a reference to an

). I discovered two places where we set the value of

: we

it out in

after calling

on it and we
both

it out and assign it in

. I decide to look in

first because the

looks innocuous enough.

In

we’re releasing the previous value of

, if any. We then go and assign the supplied

to the

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

method that takes an

. The first, taking four integer arguments, constructs and retains a new

so that isn’t our culprit. The second, taking a

, creates a

of the path

instance (which is implicitly

‘d). This one is probably good too. Next up at bat is

that takes a

as input. This one looks suspicious! Here we’re passing in the

of the

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

or

call to the

object we’re getting from

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!

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.

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:

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.

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.

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.

So… any plans to mark the millionth download of the Eclipse release this time around? Can we somehow conspire to have Denis shave his head? Can we convince Chris to get an Eclipse tattoo? I’d offer to do that myself but getting me to agree to a tattoo is hardly a worthy challenge.