diff --git a/rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/core/model/IHost.java b/rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/core/model/IHost.java index 37b8b676db8..bf58fcf69f4 100644 --- a/rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/core/model/IHost.java +++ b/rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/core/model/IHost.java @@ -12,6 +12,7 @@ * * Contributors: * Martin Oberhuber (Wind River) - [175262] IHost.getSystemType() should return IRSESystemType + * Martin Oberhuber (Wind River) - [206742] Make SystemHostPool thread-safe ********************************************************************************/ package org.eclipse.rse.core.model; @@ -87,7 +88,13 @@ public interface IHost extends IAdaptable, IRSEModelObject { * Notification method called when this connection's profile is being renamed. * Allows doing pre-death cleanup in overriders. *
+ * Implementations must not fork off other threads in the implementation of this method, + * since the old and new profiles will be locked during the rename operation so deadlock + * could occur when another thread tries to access theprofile during the time of rename + * ongoing. + *
* What we need to do is rename our entry in the preference store for our default userId. + *
*/ public void renamingSystemProfile(String oldName, String newName); diff --git a/rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/internal/core/model/SystemHostPool.java b/rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/internal/core/model/SystemHostPool.java index 4866a77a714..0805573b01e 100644 --- a/rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/internal/core/model/SystemHostPool.java +++ b/rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/internal/core/model/SystemHostPool.java @@ -19,6 +19,7 @@ * Martin Oberhuber (Wind River) - [184095] Replace systemTypeName by IRSESystemType * Martin Oberhuber (Wind River) - [186779] Fix IRSESystemType.getAdapter() * David Dykstal (IBM) - [176577] wrong enablement of "Move up/down" in connection context menu + * Martin Oberhuber (Wind River) - [206742] Make SystemHostPool thread-safe ********************************************************************************/ package org.eclipse.rse.internal.core.model; @@ -44,22 +45,31 @@ import org.eclipse.rse.internal.core.RSECoreMessages; /** * A pool of host objects. - * There is one pool per profile. - * It is named the same as its owning profile. + *+ * The host pool is tightly coupled to its owning profile: there is exactly one pool + * per profile, it always has the same name as the owning profile, and renaming it + * also renames the profile. Persistence of the host pool is also handled through + * persisting the owning profile. + *
* It is not persisted but provides a means of manipulating lists of host objects. - * Hosts are created and destroyed by the host pool so that the the relationships between the two can be maintained. + * Hosts are created and destroyed by the host pool so that the the relationships + * between the two can be maintained. + *
+ * This class is thread-safe in the sense that integrity of the host list is maintained + * even if multiple threads call multiple methods in this interface concurrently. + *
+ * @see ISystemHostPool */ public class SystemHostPool extends RSEModelObject implements ISystemHostPool { protected static final String NAME_EDEFAULT = null; - private static Hashtable pools = null; + private static Hashtable pools = new Hashtable(); private static String CONNECTION_FILE_NAME = "connection"; //$NON-NLS-1$ - protected String name = NAME_EDEFAULT; - private java.util.List connections = null; + private List connections = new ArrayList(); /** * Default constructor. @@ -74,7 +84,7 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool */ public static void reset() { - pools = null; + pools.clear(); } // ------------------------------------------------------------------------------------- @@ -82,18 +92,20 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool // ------------------------------------------------------------------------------------- /** * Return (and create if necessary) the connection pool for a given system profile. + * @param profile the profile to create a host pool for. + * @throws Exception */ public static ISystemHostPool getSystemHostPool(ISystemProfile profile) throws Exception { - if (pools == null) - pools = new Hashtable(); SystemHostPool pool = (SystemHostPool)pools.get(profile); if (pool == null) { pool = new SystemHostPool(); pool.setName(profile.getName()); try { + //FIXME Class Javadocs say that SystemHostPool is not persisted, so this should be removed! + //Or should we get the pool contents by restoring the profile instead? pool.restore(); // restore connections } catch (Exception exc) { } @@ -102,26 +114,35 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool return pool; } - /** - * Return the system profile that owns this connection pool + /* + * (non-Javadoc) + * @see org.eclipse.rse.core.model.ISystemHostPool#getSystemProfile() */ public ISystemProfile getSystemProfile() { return SystemProfileManager.getDefault().getSystemProfile(getName()); } - /** - * Rename this connection pool. + /* + * (non-Javadoc) + * @see org.eclipse.rse.core.model.ISystemHostPool#renameHostPool(java.lang.String) */ public void renameHostPool(String newName) { - IHost[] connections = getHosts(); - String oldName = getName(); - for (int idx=0; idx- * @param conn SystemConnection object to remove + /* + * (non-Javadoc) + * @see org.eclipse.rse.core.model.ISystemHostPool#deleteHost(org.eclipse.rse.core.model.IHost) */ public void deleteHost(IHost conn) { conn.deletingHost(); // let connection do any necessary cleanup - getHostList().remove(conn); + List hostList = getHostList(); + synchronized(hostList) { + hostList.remove(conn); + } setDirty(true); conn.getSystemProfile().commit(); } - /** - * Renames a given connection in the list. - * This will: - *
- * After the move, the pool containing the moved host is committed. - * @param hosts an Array of hosts to move, can be empty but must not be null. - * @param delta the amount by which to move the hosts - */ + /* + * (non-Javadoc) + * @see org.eclipse.rse.core.model.ISystemHostPool#moveHosts(org.eclipse.rse.core.model.IHost[], int) + */ public void moveHosts(IHost hosts[], int delta) { /* * Determine the indices of the supplied hosts in this pool. @@ -429,19 +443,22 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool return m * ((Integer)o1).compareTo((Integer)o2); } }); + boolean moved; List hostList = getHostList(); - for (int i = 0; i < hosts.length; i++) { - IHost host = hosts[i]; - int index = hostList.indexOf(host); - if (index >= 0) { - indices.add(new Integer(index)); + synchronized(hostList) { + for (int i = 0; i < hosts.length; i++) { + IHost host = hosts[i]; + int index = hostList.indexOf(host); + if (index >= 0) { + indices.add(new Integer(index)); + } + } + // Go through the sorted list of indices. + moved = indices.size() > 0; + for (Iterator z = indices.iterator(); z.hasNext() && moved;) { + int index = ((Integer) z.next()).intValue(); + moved &= moveHost(hostList, index, delta); } - } - // Go through the sorted list of indices. - boolean moved = indices.size() > 0; - for (Iterator z = indices.iterator(); z.hasNext() && moved;) { - int index = ((Integer) z.next()).intValue(); - moved &= moveHost(hostList, index, delta); } if (moved) { invalidateCache(); @@ -450,7 +467,7 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool } /** - * Move a host to a new location in the host pool. + * Move a host to a new location in the passed-in host list. * @param hostList the list of hosts to modify * @param oldPos the index of the host to move. If outside the bounds of the list, the list is not altered. * @param delta the amount by which to move the host. If the resulting @@ -471,19 +488,29 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool return moved; } - /** - * Order connections according to user preferences. - * Called after restore. + /* + * (non-Javadoc) + * @see org.eclipse.rse.core.model.ISystemHostPool#orderHosts(java.lang.String[]) */ public void orderHosts(String[] names) { List connList = getHostList(); - IHost[] conns = new IHost[names.length]; - for (int idx = 0; idx < conns.length; idx++) { - conns[idx] = getHost(names[idx]); - } - connList.clear(); - for (int idx = 0; idx < conns.length; idx++) { - connList.add(conns[idx]); + synchronized(connList) { + //Threading: need to call getHost() from inside the synchronized block in order + //to avoid problems with adding/removing hosts while the re-ordering is taking + //place... hosts added during the re-ordering could otherwise be deleted. + //In order to not have an alien method call here, getHost() is declared final. + IHost[] conns = new IHost[names.length]; + for (int idx = 0; idx < conns.length; idx++) { + conns[idx] = getHost(names[idx]); //may return null host + } + //TODO what should we do with hosts that are not in the name list? + //Currently, these will be removed... should they be added at the end instead? + connList.clear(); + for (int idx = 0; idx < conns.length; idx++) { + if (conns[idx]!=null) { + connList.add(conns[idx]); + } + } } invalidateCache(); } @@ -503,6 +530,7 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool { return getRootSaveFileName(connection.getAliasName()); } + /** * Return the root save file name without the extension .xmi */ @@ -521,8 +549,9 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool return getName(); } - /** - * @generated This field/method will be replaced during code generation + /* + * (non-Javadoc) + * @see org.eclipse.rse.core.model.IRSEModelObject#getName() */ public String getName() { @@ -535,20 +564,41 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool } /** - * @generated This field/method will be replaced during code generation. + * Set the name of this host pool. + *
+ * This method should not be called by clients directly in order + * to maintain correct relationship to the owning profile. Clients + * should call {@link #renameHostPool(String)} instead. + *
+ * @param newName The new value of the Name attribute. */ - public void setName(String newName) + protected void setName(String newName) { name = newName; } - - public List getHostList() + /** + * Return the internal list of connection references. + * + * The list returned is considered internal and not synchronized. + * Modifications will directly apply to any other client or thread. + * Therefore, users of the returned host list should typically be + * synchronized to avoid modification of the list through another thread + * when working on it. + * + * Clients are not expected to get a handle on this list and + * are expected to use @link{#getHosts()} instead. + * + * Subclasses may override this method to perform additional work + * or return a different kind of List implementation, but they must + * ensure that they always return the same List object (because + * clients will synchronize on that object). In other words, the + * List may be modified but never totally exchanged. + * + * @return The internal list of connection references. + */ + protected List getHostList() { - if (connections == null) - { - connections = new ArrayList(); - } return connections; } diff --git a/rse/plugins/org.eclipse.rse.ui/model/org/eclipse/rse/ui/internal/model/SystemRegistry.java b/rse/plugins/org.eclipse.rse.ui/model/org/eclipse/rse/ui/internal/model/SystemRegistry.java index 28e774f3786..2634d6aae0f 100644 --- a/rse/plugins/org.eclipse.rse.ui/model/org/eclipse/rse/ui/internal/model/SystemRegistry.java +++ b/rse/plugins/org.eclipse.rse.ui/model/org/eclipse/rse/ui/internal/model/SystemRegistry.java @@ -36,6 +36,7 @@ * Martin Oberhuber (Wind River) - [165674] Sort subsystem configurations by priority then Id * Martin Oberhuber (Wind River) - [194898] Avoid NPE when doing EVENT_REFRESH_REMOTE on a subsys without filters * David McKnight (IBM) - [207100] adding ISystemRegistry.isRegisteredSystemRemoteChangeListener + * Martin Oberhuber (Wind River) - [206742] Make SystemHostPool thread-safe ********************************************************************************/ package org.eclipse.rse.ui.internal.model; @@ -1392,22 +1393,25 @@ public class SystemRegistry implements ISystemRegistry } /** - * Return all connections in all active profiles. Never returns null, but may return a zero-length array. + * Return all connections in all active profiles. + * Never returns null, but may return a zero-length array. + * All array elements are valid hosts (never returns null elements). */ public IHost[] getHosts() { ISystemHostPool[] pools = getHostPools(); - Vector v = new Vector(); - for (int idx = 0; idx < pools.length; idx++) - { - IHost[] conns = getHostsByProfile(getSystemProfile(pools[idx])); - if (conns != null) - for (int jdx = 0; jdx < conns.length; jdx++) - v.addElement(conns[jdx]); + List hosts = new ArrayList(); + for (int idx = 0; idx < pools.length; idx++) { + IHost[] conns = pools[idx].getHosts(); + if (conns != null) { + for (int jdx = 0; jdx < conns.length; jdx++) { + //ISystemHostPool ensures that we never have "null" hosts. + assert conns[jdx]!=null : "Null host in pool "+pools[idx].getName()+" at "+jdx; //$NON-NLS-1$ //$NON-NLS-2$ + hosts.add(conns[jdx]); + } + } } - IHost[] allConns = new IHost[v.size()]; - for (int idx = 0; idx < v.size(); idx++) - allConns[idx] = (IHost) v.elementAt(idx); + IHost[] allConns = (IHost[])hosts.toArray(new IHost[hosts.size()]); return allConns; }