Skip to main content

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


Hi Martin,

Thanks for the tip with this.  I had to alter your stuff a little because the IRSECallback.done() method occurs on the job thread and the callback needs to happen in the UI thread because it touches widgets.  I changed things so that done calls Display.asyncExec() to do the response UI processing.

____________________________________
David McKnight    
Phone:   905-413-3902 , T/L:  969-3902
Internet: dmcknigh@xxxxxxxxxx
Mail:       D1/YFY/8200/TOR
____________________________________



"Oberhuber, Martin" <Martin.Oberhuber@xxxxxxxxxxxxx>

25/05/2007 12:23 PM

To
David McKnight/Toronto/IBM@IBMCA
cc
"Target Management developer discussions" <dsdp-tm-dev@xxxxxxxxxxx>
Subject
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