Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
[dsdp-tm-dev] Review: SystemTableViewPart.java v1.12

Hi Dave,

In SystemTableViewPart.java v1.12 you made this change:
call connect with monitor instead of the deprecated method	

But you call connect(monitor) inside a UI Job (method
runInUIThread). This is BAD and really not what we want,
since connect() should always be done asynchronously.

The old deprecated connect() method which you changed,
would have accounted for this by creating a backround
worker job automatically; this was also not quite 
correct in terms of what the code should do, but after
all it could not connect in the UI thread which might
eventually block all of Eclipse!

What you need to do, is call the 
   connect(boolean, IRSECallback)
method, with a callback registered to do the work that
needs to be done once connect is complete.

Attached is a patch to do just that. 
I wrote it for you to better understand, but
did not test it. Apply as you see fit.

Thanks,
--
Martin Oberhuber
Wind River Systems, Inc.
Target Management Project Lead, DSDP PMC Member
http://www.eclipse.org/dsdp/tm
### Eclipse Workspace Patch 1.0
#P org.eclipse.rse.ui
Index: UI/org/eclipse/rse/internal/ui/view/SystemTableViewPart.java
===================================================================
RCS file: /cvsroot/dsdp/org.eclipse.tm.rse/plugins/org.eclipse.rse.ui/UI/org/eclipse/rse/internal/ui/view/SystemTableViewPart.java,v
retrieving revision 1.12
diff -u -r1.12 SystemTableViewPart.java
--- UI/org/eclipse/rse/internal/ui/view/SystemTableViewPart.java	25 May 2007 15:23:46 -0000	1.12
+++ UI/org/eclipse/rse/internal/ui/view/SystemTableViewPart.java	25 May 2007 16:21:18 -0000
@@ -55,6 +55,7 @@
 import org.eclipse.rse.core.events.SystemResourceChangeEvent;
 import org.eclipse.rse.core.filters.ISystemFilterReference;
 import org.eclipse.rse.core.model.IHost;
+import org.eclipse.rse.core.model.IRSECallback;
 import org.eclipse.rse.core.model.ISystemContainer;
 import org.eclipse.rse.core.model.ISystemProfile;
 import org.eclipse.rse.core.model.ISystemRegistry;
@@ -634,14 +635,14 @@
 			_rmemento = memento;
 		}
 
-		public IStatus runInUIThread(IProgressMonitor monitor)
+		public IStatus runInUIThread(final IProgressMonitor monitor)
 		{
-			IMemento memento = _rmemento;
+			final IMemento memento = _rmemento;
 			String profileId = memento.getString(TAG_TABLE_VIEW_PROFILE_ID);
 			String connectionId = memento.getString(TAG_TABLE_VIEW_CONNECTION_ID);
 			String subsystemId = memento.getString(TAG_TABLE_VIEW_SUBSYSTEM_ID);
-			String filterID = memento.getString(TAG_TABLE_VIEW_FILTER_ID);
-			String objectID = memento.getString(TAG_TABLE_VIEW_OBJECT_ID);
+			final String filterID = memento.getString(TAG_TABLE_VIEW_FILTER_ID);
+			final String objectID = memento.getString(TAG_TABLE_VIEW_OBJECT_ID);
 
 			ISystemRegistryUI registryUI = RSEUIPlugin.getTheSystemRegistryUI();
 			ISystemRegistry registry = RSECorePlugin.getTheSystemRegistry();
@@ -665,62 +666,61 @@
 			else
 			{
 				// from the subsystem ID determine the profile, system and subsystem
-				ISubSystem subsystem = registry.getSubSystem(subsystemId);
+				final ISubSystem subsystem = registry.getSubSystem(subsystemId);
 
-				if (subsystem != null)
-				{
-					if (filterID == null && objectID == null)
-					{
+				if (subsystem != null) {
+					if (filterID == null && objectID == null) {
 						input = subsystem;
 					}
-					else
-					{
-
-						if (!subsystem.isConnected())
-						{
-							try
-							{
-								subsystem.connect(monitor, false);
+					else {
+						if (!subsystem.isConnected()) {
+							try {
+								final Object finInput = input;
+								subsystem.connect(false, new IRSECallback() {
+									public void done(IStatus status, Object result) {
+										runOnceConnected(monitor, finInput, memento, subsystem, filterID, objectID);
+									}
+								});
+								return Status.OK_STATUS;
 							}
-							catch (Exception e)
-							{
+							catch (Exception e) {
 								return Status.CANCEL_STATUS;
 							}
 						}
-						if (subsystem.isConnected())
-						{
-
-							if (filterID != null)
-							{
-								try
-								{
-									input = subsystem.getObjectWithAbsoluteName(filterID);
-								}
-								catch (Exception e)
-								{
-								}
-							}
-							else
-							{
-
-								if (objectID != null)
-								{
-
-									try
-									{
-										input = subsystem.getObjectWithAbsoluteName(objectID);
-									}
-									catch (Exception e)
-									{
-										return Status.CANCEL_STATUS;
-									}
-								}
-							} // end else
-						} // end if (subsystem.isConnected)
+						return runOnceConnected(monitor, input, memento, subsystem, filterID, objectID);
 					} // end else
 				} // end if (subsystem != null)
 			} // end else
+			return runWithInput(monitor, input, memento);
+		}
+			
+		public IStatus runOnceConnected(IProgressMonitor monitor, Object input, IMemento memento, ISubSystem subsystem, String filterID, String objectID)
+		{
+			if (subsystem.isConnected()) {
+				if (filterID != null) {
+					try {
+						input = subsystem.getObjectWithAbsoluteName(filterID);
+					}
+					catch (Exception e) {
+						//ignore
+					}
+				}
+				else {
+					if (objectID != null) {
+						try {
+							input = subsystem.getObjectWithAbsoluteName(objectID);
+						}
+						catch (Exception e)	{
+							return Status.CANCEL_STATUS;
+						}
+					}
+				} // end else
+			} // end if (subsystem.isConnected)
+			return runWithInput(monitor, input, memento);
+		}
 
+		public IStatus runWithInput(IProgressMonitor monitor, Object input, IMemento memento)
+		{
 			if (input != null && input instanceof IAdaptable)
 			{
 				_mementoInput = (IAdaptable) input;

Back to the top