mirrors/cdt
1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-08-20 22:55:51 +02:00
Code Issues Wiki Activity

[206742] Make SystemHostPool thread-safe

Browse source
This commit is contained in:
Martin Oberhuber 2007-11-21 16:39:23 +00:00
parent 430986ffb5
commit 0fbab59e21
3 changed files with 238 additions and 177 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/core/model/IHost.java
View file

@ -12,6 +12,7 @@
* *
* Contributors: * Contributors:
* Martin Oberhuber (Wind River) - [175262] IHost.getSystemType() should return IRSESystemType * 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; 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. * Notification method called when this connection's profile is being renamed.
* Allows doing pre-death cleanup in overriders. * Allows doing pre-death cleanup in overriders.
* <p> * <p>
* 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.
* </p><p>
* What we need to do is rename our entry in the preference store for our default userId. * What we need to do is rename our entry in the preference store for our default userId.
* </p>
*/ */
public void renamingSystemProfile(String oldName, String newName); public void renamingSystemProfile(String oldName, String newName);

382
rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/internal/core/model/SystemHostPool.java
View file

@ -19,6 +19,7 @@
* Martin Oberhuber (Wind River) - [184095] Replace systemTypeName by IRSESystemType * Martin Oberhuber (Wind River) - [184095] Replace systemTypeName by IRSESystemType
* Martin Oberhuber (Wind River) - [186779] Fix IRSESystemType.getAdapter() * Martin Oberhuber (Wind River) - [186779] Fix IRSESystemType.getAdapter()
* David Dykstal (IBM) - [176577] wrong enablement of "Move up/down" in connection context menu * 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; package org.eclipse.rse.internal.core.model;
@ -44,22 +45,31 @@ import org.eclipse.rse.internal.core.RSECoreMessages;
/** /**
* A pool of host objects. * A pool of host objects.
* There is one pool per profile. * <p>
* 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.
* </p><p>
* It is not persisted but provides a means of manipulating lists of host objects. * 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.
* </p><p>
* 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.
* </p>
* @see ISystemHostPool
*/ */
public class SystemHostPool extends RSEModelObject implements ISystemHostPool public class SystemHostPool extends RSEModelObject implements ISystemHostPool
{ {
protected static final String NAME_EDEFAULT = null; 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$ private static String CONNECTION_FILE_NAME = "connection"; //$NON-NLS-1$
protected String name = NAME_EDEFAULT; protected String name = NAME_EDEFAULT;
private java.util.List connections = null; private List connections = new ArrayList();
/** /**
* Default constructor. * Default constructor.
@ -74,7 +84,7 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool
*/ */
public static void reset() 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. * 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) public static ISystemHostPool getSystemHostPool(ISystemProfile profile)
throws Exception throws Exception
{ {
if (pools == null)
pools = new Hashtable();
SystemHostPool pool = (SystemHostPool)pools.get(profile); SystemHostPool pool = (SystemHostPool)pools.get(profile);
if (pool == null) if (pool == null)
{ {
pool = new SystemHostPool(); pool = new SystemHostPool();
pool.setName(profile.getName()); pool.setName(profile.getName());
try { 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 pool.restore(); // restore connections
} catch (Exception exc) { } catch (Exception exc) {
} }
@ -102,26 +114,35 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool
return pool; return pool;
} }
/** /*
* Return the system profile that owns this connection pool * (non-Javadoc)
* @see org.eclipse.rse.core.model.ISystemHostPool#getSystemProfile()
*/ */
public ISystemProfile getSystemProfile() public ISystemProfile getSystemProfile()
{ {
return SystemProfileManager.getDefault().getSystemProfile(getName()); 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) public void renameHostPool(String newName)
{ {
IHost[] connections = getHosts(); //Threading: We need to ensure that new hosts are not added while the rename is
String oldName = getName(); //ongoing. Therefore, we lock even though renamingSystemProfile() is an alien
for (int idx=0; idx<connections.length; idx++) //method -- Javadoc in that method warns about the possible deadlock.
{ List hostList = getHostList();
connections[idx].renamingSystemProfile(oldName, newName); synchronized(hostList) {
String oldName = getName();
Iterator it = hostList.iterator();
while (it.hasNext()) {
IHost curHost = (IHost)it.next();
curHost.renamingSystemProfile(oldName, newName);
}
setName(newName);
} }
setName(newName);
} }
@ -131,24 +152,26 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool
*/ */
public void printConnections() public void printConnections()
{ {
java.util.List conns = getHostList(); List conns = getHostList();
Iterator connsList = conns.iterator(); synchronized(conns) {
if (!connsList.hasNext()) Iterator connsList = conns.iterator();
{ if (!connsList.hasNext())
System.out.println(); {
System.out.println("No connections"); //$NON-NLS-1$ System.out.println();
} System.out.println("No connections"); //$NON-NLS-1$
while (connsList.hasNext()) }
{ while (connsList.hasNext())
System.out.println(); {
IHost conn = (IHost)connsList.next(); System.out.println();
System.out.println(" AliasName.....: " + conn.getAliasName()); //$NON-NLS-1$ IHost conn = (IHost)connsList.next();
System.out.println(" -----------------------------------------------------"); //$NON-NLS-1$ System.out.println(" AliasName.....: " + conn.getAliasName()); //$NON-NLS-1$
System.out.println(" HostName......: " + conn.getHostName()); //$NON-NLS-1$ System.out.println(" -----------------------------------------------------"); //$NON-NLS-1$
System.out.println(" SystemType....: " + conn.getSystemType().getId()); //$NON-NLS-1$ System.out.println(" HostName......: " + conn.getHostName()); //$NON-NLS-1$
System.out.println(" Description...: " + conn.getDescription()); //$NON-NLS-1$ System.out.println(" SystemType....: " + conn.getSystemType().getId()); //$NON-NLS-1$
System.out.println(" UserId........: " + conn.getDefaultUserId()); //$NON-NLS-1$ System.out.println(" Description...: " + conn.getDescription()); //$NON-NLS-1$
} System.out.println(" UserId........: " + conn.getDefaultUserId()); //$NON-NLS-1$
}
}
} }
// ------------------------------------------------------------------------------------- // -------------------------------------------------------------------------------------
@ -198,8 +221,8 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool
conn = systemType.createNewHostInstance(profile); conn = systemType.createNewHostInstance(profile);
} }
// Fallback to create host object instance here if failed by system type provider. // Fallback to create host object instance here if failed by system type provider.
if (conn == null) conn = new Host(profile);
assert conn != null; assert conn != null;
if (conn == null) conn = new Host(profile);
addHost(conn); // only record internally if saved successfully addHost(conn); // only record internally if saved successfully
conn.setHostPool(this); conn.setHostPool(this);
@ -258,19 +281,20 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool
commit(conn); commit(conn);
} }
/*
/** * (non-Javadoc)
* Return array of connections in this pool * @see org.eclipse.rse.core.model.ISystemHostPool#getHosts()
*/ */
public IHost[] getHosts() public IHost[] getHosts()
{ {
//Must be synchronized in order to avoid change of host list size while populating the array
return (IHost[])getHostList().toArray(new IHost[connections.size()]); List conns = getHostList();
synchronized(conns) {
return (IHost[])conns.toArray(new IHost[conns.size()]);
}
} }
/**
/*
* Invalidate cache so it will be regenerated * Invalidate cache so it will be regenerated
*/ */
protected void invalidateCache() protected void invalidateCache()
@ -278,127 +302,121 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool
setDirty(true); setDirty(true);
} }
/** /*
* Return a connection object, given its alias name. * (non-Javadoc)
* Can be used to test if an alias name is already used (non-null return). * @see org.eclipse.rse.core.model.ISystemHostPool#getHost(java.lang.String)
* @param aliasName unique aliasName (case insensitive) to search on.
* @return SystemConnection object with unique aliasName, or null if
* no connection object with this name exists.
*/ */
public IHost getHost(String aliasName) public final IHost getHost(String aliasName)
{ {
IHost conn = null; //This method is final because it is called from inside the synchronized block
IHost currconn = null; //in the orderHosts() method. Also, if subclasses want to override the way how
java.util.List conns = getHostList(); //hosts are returned and compared, they better need to override the List
Iterator i = conns.iterator(); //implementation that's returned by getHostList().
while (i.hasNext() && (conn==null)) List conns = getHostList();
{ synchronized(conns) {
currconn = (IHost)i.next(); Iterator i = conns.iterator();
if (currconn.getAliasName().equalsIgnoreCase(aliasName)) while (i.hasNext())
conn = currconn; {
IHost currconn = (IHost)i.next();
if (currconn.getAliasName().equalsIgnoreCase(aliasName))
return currconn;
}
} }
return conn; return null;
} }
/**
* Return the connection at the given zero-based offset /*
* (non-Javadoc)
* @see org.eclipse.rse.core.model.ISystemHostPool#getHost(int)
*/ */
public IHost getHost(int pos) public final IHost getHost(int pos)
{ {
java.util.List conns = getHostList(); List conns = getHostList();
if (pos < conns.size()) synchronized(conns) {
return (IHost)conns.get(pos); if (pos < conns.size())
else return (IHost)conns.get(pos);
return null; else
return null;
}
} }
/**
* Return the zero-based position of a SystemConnection object within its profile. /*
* (non-Javadoc)
* @see org.eclipse.rse.core.model.ISystemHostPool#getHostPosition(org.eclipse.rse.core.model.IHost)
*/ */
public int getHostPosition(IHost conn) public int getHostPosition(IHost conn)
{ {
int position = -1; List hostList = getHostList();
boolean match = false; synchronized(hostList) {
java.util.List conns = getHostList(); return hostList.indexOf(conn);
Iterator i = conns.iterator();
int idx = 0;
while (!match && i.hasNext())
{
IHost currConn = (IHost)i.next();
if (conn.equals(currConn))
{
match = true;
position = idx;
}
idx++;
} }
return position;
} }
/** /*
* Return the number of SystemConnection objects within this pool. * (non-Javadoc)
* @see org.eclipse.rse.core.model.ISystemHostPool#getHostCount()
*/ */
public int getHostCount() public int getHostCount()
{ {
java.util.List conns = getHostList(); List conns = getHostList();
return conns.size(); synchronized(conns) {
return conns.size();
}
} }
public boolean addHost(IHost conn) public boolean addHost(IHost conn)
{ {
List hostList = getHostList(); assert conn!=null;
if (!hostList.contains(conn)) if (conn!=null) {
{ List hostList = getHostList();
hostList.add(conn); synchronized(hostList) {
if (!hostList.contains(conn))
{
hostList.add(conn);
}
}
conn.setHostPool(this);
invalidateCache();
return true;
} }
conn.setHostPool(this); return false;
invalidateCache();
return true;
} }
/** /*
* Removes a given connection from the list and deletes it from disk. * (non-Javadoc)
* <p> * @see org.eclipse.rse.core.model.ISystemHostPool#deleteHost(org.eclipse.rse.core.model.IHost)
* This will:
* <ul>
* <li>Delete the connection in memory
* <li>Delete the underlying folder
* </ul>
* <p>
* @param conn SystemConnection object to remove
*/ */
public void deleteHost(IHost conn) public void deleteHost(IHost conn)
{ {
conn.deletingHost(); // let connection do any necessary cleanup conn.deletingHost(); // let connection do any necessary cleanup
getHostList().remove(conn); List hostList = getHostList();
synchronized(hostList) {
hostList.remove(conn);
}
setDirty(true); setDirty(true);
conn.getSystemProfile().commit(); conn.getSystemProfile().commit();
} }
/** /*
* Renames a given connection in the list. * (non-Javadoc)
* This will: * @see org.eclipse.rse.core.model.ISystemHostPool#renameHost(org.eclipse.rse.core.model.IHost, java.lang.String)
* <ul>
* <li>Rename the profile in memory
* <li>Rename the underlying folder
* <li>Update the user preferences if this profile is currently active.
* </ul>
* @param conn SystemConnection object to rename
* @param newName The new name to give that connection.
*/ */
public void renameHost(IHost conn, String newName) public void renameHost(IHost conn, String newName)
throws Exception throws Exception
{ {
conn.setAliasName(newName); //must not change the alias name while a getHost() or orderHosts() is ongoing
synchronized(connections) {
conn.setAliasName(newName);
}
invalidateCache(); invalidateCache();
commit(conn); commit(conn);
} }
/** /*
* Duplicates a given connection in this list within this list or another list. * (non-Javadoc)
* @param targetPool The SystemConnectionPool to hold the copied connection. Can equal this connection, as long as alias name is unique * @see org.eclipse.rse.core.model.ISystemHostPool#cloneHost(org.eclipse.rse.core.model.ISystemHostPool, org.eclipse.rse.core.model.IHost, java.lang.String)
* @param conn SystemConnection object (within our pool) to clone
* @param aliasName New, unique, alias name to give this connection. Clone will fail if this is not unique.
*/ */
public IHost cloneHost(ISystemHostPool targetPool, IHost conn, String aliasName) public IHost cloneHost(ISystemHostPool targetPool, IHost conn, String aliasName)
throws Exception throws Exception
@ -409,14 +427,10 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool
return copy; return copy;
} }
/** /*
* Move existing hosts a given number of positions in the same pool. * (non-Javadoc)
* If the delta is negative, they are all moved up (left) by the given amount. If * @see org.eclipse.rse.core.model.ISystemHostPool#moveHosts(org.eclipse.rse.core.model.IHost[], int)
* positive, they are all moved down (right) by the given amount.<p> */
* 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
*/
public void moveHosts(IHost hosts[], int delta) { public void moveHosts(IHost hosts[], int delta) {
/* /*
* Determine the indices of the supplied hosts in this pool. * 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); return m * ((Integer)o1).compareTo((Integer)o2);
} }
}); });
boolean moved;
List hostList = getHostList(); List hostList = getHostList();
for (int i = 0; i < hosts.length; i++) { synchronized(hostList) {
IHost host = hosts[i]; for (int i = 0; i < hosts.length; i++) {
int index = hostList.indexOf(host); IHost host = hosts[i];
if (index >= 0) { int index = hostList.indexOf(host);
indices.add(new Integer(index)); 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) { if (moved) {
invalidateCache(); 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 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 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 * @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; return moved;
} }
/** /*
* Order connections according to user preferences. * (non-Javadoc)
* Called after restore. * @see org.eclipse.rse.core.model.ISystemHostPool#orderHosts(java.lang.String[])
*/ */
public void orderHosts(String[] names) { public void orderHosts(String[] names) {
List connList = getHostList(); List connList = getHostList();
IHost[] conns = new IHost[names.length]; synchronized(connList) {
for (int idx = 0; idx < conns.length; idx++) { //Threading: need to call getHost() from inside the synchronized block in order
conns[idx] = getHost(names[idx]); //to avoid problems with adding/removing hosts while the re-ordering is taking
} //place... hosts added during the re-ordering could otherwise be deleted.
connList.clear(); //In order to not have an alien method call here, getHost() is declared final.
for (int idx = 0; idx < conns.length; idx++) { IHost[] conns = new IHost[names.length];
connList.add(conns[idx]); 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(); invalidateCache();
} }
@ -503,6 +530,7 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool
{ {
return getRootSaveFileName(connection.getAliasName()); return getRootSaveFileName(connection.getAliasName());
} }
/** /**
* Return the root save file name without the extension .xmi * Return the root save file name without the extension .xmi
*/ */
@ -521,8 +549,9 @@ public class SystemHostPool extends RSEModelObject implements ISystemHostPool
return getName(); 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() 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.
* <p>
* 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.
* </p>
* @param newName The new value of the Name attribute.
*/ */
public void setName(String newName) protected void setName(String newName)
{ {
name = 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; return connections;
} }

26
rse/plugins/org.eclipse.rse.ui/model/org/eclipse/rse/ui/internal/model/SystemRegistry.java
View file

@ -36,6 +36,7 @@
* Martin Oberhuber (Wind River) - [165674] Sort subsystem configurations by priority then Id * 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 * Martin Oberhuber (Wind River) - [194898] Avoid NPE when doing EVENT_REFRESH_REMOTE on a subsys without filters
* David McKnight (IBM) - [207100] adding ISystemRegistry.isRegisteredSystemRemoteChangeListener * David McKnight (IBM) - [207100] adding ISystemRegistry.isRegisteredSystemRemoteChangeListener
* Martin Oberhuber (Wind River) - [206742] Make SystemHostPool thread-safe
********************************************************************************/ ********************************************************************************/
package org.eclipse.rse.ui.internal.model; 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() public IHost[] getHosts()
{ {
ISystemHostPool[] pools = getHostPools(); ISystemHostPool[] pools = getHostPools();
Vector v = new Vector(); List hosts = new ArrayList();
for (int idx = 0; idx < pools.length; idx++) for (int idx = 0; idx < pools.length; idx++) {
{ IHost[] conns = pools[idx].getHosts();
IHost[] conns = getHostsByProfile(getSystemProfile(pools[idx])); if (conns != null) {
if (conns != null) for (int jdx = 0; jdx < conns.length; jdx++) {
for (int jdx = 0; jdx < conns.length; jdx++) //ISystemHostPool ensures that we never have "null" hosts.
v.addElement(conns[jdx]); 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()]; IHost[] allConns = (IHost[])hosts.toArray(new IHost[hosts.size()]);
for (int idx = 0; idx < v.size(); idx++)
allConns[idx] = (IHost) v.elementAt(idx);
return allConns; return allConns;
} }