Bug 225089

Summary:	[ssh][shells][api] Canceling connection leads to exception
Product:	[Tools] Target Management	Reporter:	Benjamin Muskalla <b.muskalla>
Component:	RSE	Assignee:	David Dykstal <ddykstal.eclipse>
Status:	RESOLVED FIXED	QA Contact:	Martin Oberhuber <mober.at+eclipse>
Severity:	normal
Priority:	P3	CC:	ddykstal.eclipse
Version:	unspecified	Keywords:	api
Target Milestone:	3.0 M6
Hardware:	PC
OS:	Linux
Whiteboard:

Description Benjamin Muskalla

2008-04-01 09:43:09 EDT

I've a linux host with ssh connect service.
When using the "Connect" menu item for "SSH Shells" i get a dialog to log in. If I click cancel the following exception occurs. As canceling the dialog is a regular user operation it should not throw any exception.

-- Error Log --
Date: Tue Apr 01 15:38:00 CEST 2008
Message: java.lang.InterruptedException
Severity: Error
Plugin ID: org.eclipse.rse.ui
Stack Trace:
java.lang.InterruptedException
at org.eclipse.rse.ui.subsystems.StandardCredentialsProvider.promptForCredentials(StandardCredentialsProvider.java:389)
at org.eclipse.rse.ui.subsystems.StandardCredentialsProvider.acquireCredentials(StandardCredentialsProvider.java:218)
at org.eclipse.rse.core.subsystems.AuthenticatingConnectorService.acquireCredentials(AuthenticatingConnectorService.java:188)
at org.eclipse.rse.core.subsystems.SubSystem.promptForPassword(SubSystem.java:2352)
at org.eclipse.rse.core.subsystems.SubSystem$1.run(SubSystem.java:2254)
at org.eclipse.ui.internal.UILockListener.doPendingWork(UILockListener.java:155)
at org.eclipse.ui.internal.UISynchronizer$3.run(UISynchronizer.java:144)
at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:35)
at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:130)
at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:3323)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:2985)
at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:2375)
at org.eclipse.ui.internal.Workbench.runUI(Workbench.java:2339)
at org.eclipse.ui.internal.Workbench.access$4(Workbench.java:2205)
at org.eclipse.ui.internal.Workbench$4.run(Workbench.java:478)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:288)
at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:473)
at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:149)
at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:106)
at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:193)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:110)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:79)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:362)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:175)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:549)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:504)
at org.eclipse.equinox.launcher.Main.run(Main.java:1236)

Comment 1 David Dykstal

2008-04-01 10:03:40 EDT

Martin --

I believe we were considering changing this to throw an OperationCanceledException and have the subsystem handle it properly. This should probably be done for M6.

-- Dave

Comment 2 Martin Oberhuber

2008-04-01 10:23:29 EDT

Yes I agree that OperationCanceledException is better than InterruptedException.

What's interesting is that bug 176605 seems related but it's actually different: that one runs into "Auth Cancel" whereas this one runs into InterruptedException. Looks like this is specific to the "Launch Shell" command.

Dave can you look into this? I'm currently swamped with reviewing patches.

Comment 3 David Dykstal

2008-04-01 10:47:12 EDT

I'll take a look at it. I don't think this should be very hard.

Comment 4 David Dykstal

2008-04-04 09:21:12 EDT

API changes (these are apparently non-breaking according to the API checker)
ICredentialsProvider
acquireCredentials(boolean reacquire) throws OperationCanceledException rather than InterruptedException
repairCredentials(SystemMessage message)throws OperationCanceledException rather than InterruptedException

IConnectorService
acquireCredentials(boolean refresh) throws OperationCanceledException rather than InterruptedException

Several classes were updated to handle or rethrow OperationCanceledException rather than InterruptedException.

Comment 5 Martin Oberhuber

2008-04-04 09:27:12 EDT

Changing checked exceptions thrown does not break binary compatibility, but it does break source compatibility and contract compatibility:
   http://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_interfaces_-_API_methods

I guess the API Tooling won't mark these either because it's only dealing with binary compatibility alone (for now), or because most of our plugins already do break binary compatibility, going from 2.0 to 3.0, so there is no use in separately marking them.

Personally, I would recommend marking your change of checked exception thrown with an "@since 3.0 throwing OperationCanceledException"
when API Methods are concerned (no need doing so in the internal classes).

Comment 6 David Dykstal

2008-04-04 09:35:49 EDT

Good idea. I've committed the original change, but will add the @since tags and recommit.

Comment 7 David Dykstal

2008-04-04 10:03:55 EDT

@since tags committed.