[439545][dstore] potential deadlock on senders during shutdown

rse/plugins/org.eclipse.dstore.core/src/org/eclipse/dstore/core/server/ConnectionEstablisher.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2002, 2012 IBM Corporation and others.
+ * Copyright (c) 2002, 2014 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -31,6 +31,7 @@
  * David McKnight   (IBM) - [378136] [dstore] miner.finish is stuck
  * David McKnight    (IBM) - [388472] [dstore] need alternative option for getting at server hostname
  * David McKnight   (IBM)  - [390681] [dstore] need to merge differences between HEAD stream and 3.2 in ConnectionEstablisher.finished()
+ * David McKnight  (IBM)   [439545][dstore] potential deadlock on senders during shutdown
  *******************************************************************************/
 
 package org.eclipse.dstore.core.server;
@@ -224,10 +225,6 @@ public class ConnectionEstablisher
 		if (_dataStore.getClient() != null) {
 			_dataStore.getClient().getLogger().logInfo(this.getClass().toString(), "ConnectionEstablisher.finished()"); //$NON-NLS-1$
 		}
-		if (_dataStore.getClient() != null) {
-			_dataStore.getClient().getLogger().logInfo(this.getClass().toString(), "ConnectionEstablisher - removing sender"); //$NON-NLS-1$
-		}
-		_updateHandler.removeSenderWith(receiver.socket());
 
 		if (_dataStore.getClient() != null) {
 			_dataStore.getClient().getLogger().logInfo(this.getClass().toString(), "ConnectionEstablisher - removing receiver"); //$NON-NLS-1$
@@ -248,6 +245,12 @@ public class ConnectionEstablisher
 			}
 			_updateHandler.finish();
 			
+			if (_dataStore.getClient() != null) {
+				_dataStore.getClient().getLogger().logInfo(this.getClass().toString(), "ConnectionEstablisher - removing sender"); //$NON-NLS-1$
+			}
+			_updateHandler.removeSenderWith(receiver.socket());
+			
+			
 			if (_dataStore.getClient() != null) {
 				_dataStore.getClient().getLogger().logInfo(this.getClass().toString(), "ConnectionEstablisher - finishing DataStore"); //$NON-NLS-1$
 			}

rse/plugins/org.eclipse.dstore.core/src/org/eclipse/dstore/internal/core/server/ServerUpdateHandler.java

@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2002, 2013 IBM Corporation and others.
+ * Copyright (c) 2002, 2014 IBM Corporation and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -18,6 +18,7 @@
  * David McKnight    (IBM)  - [358301] [DSTORE] Hang during debug source look up
  * David McKnight  (IBM)   [388873][dstore] ServerUpdateHandler _senders list should be synchronized
  * David McKnight  (IBM)   [404082][dstore] race condition on finish, removing senders
+ * David McKnight  (IBM)   [439545][dstore] potential deadlock on senders during shutdown
  *******************************************************************************/
 
 package org.eclipse.dstore.internal.core.server;
@@ -41,6 +42,7 @@ import org.eclipse.dstore.internal.core.util.Sender;
 public class ServerUpdateHandler extends UpdateHandler
 {
 
+	private Sender _primarySender; // there should really only be one
 
 	private ArrayList _senders;
 	private CommandGenerator _commandGenerator;
@@ -164,6 +166,7 @@ public class ServerUpdateHandler extends UpdateHandler
 		synchronized(_senders){
 			_senders.add(sender);
 		}
+		_primarySender = sender;
 	}
 
 	/**
@@ -175,6 +178,9 @@ public class ServerUpdateHandler extends UpdateHandler
 		synchronized (_senders){
 			_senders.remove(sender);
 		}
+		if (sender == _primarySender){
+			_primarySender = null;
+		}
 		if (_senders.size() == 0)
 		{
 			finish();
@@ -214,13 +220,7 @@ public class ServerUpdateHandler extends UpdateHandler
 		document.setPendingTransfer(true);
 		document.setParent(null);
 		
-		synchronized (_senders){
+		_primarySender.sendFile(document, bytes, size, binary);
-			for (int j = 0; j < _senders.size(); j++)
-			{
-				Sender sender = (Sender) _senders.get(j);
-				sender.sendFile(document, bytes, size, binary);
-			}
-		}
 	}
 
 	/**
@@ -254,13 +254,8 @@ public class ServerUpdateHandler extends UpdateHandler
 		document.setAttribute(DE.A_SOURCE, path);
 		document.setPendingTransfer(true);
 		document.setParent(null);
-		synchronized (_senders){
+		
-			for (int j = 0; j < _senders.size(); j++)
+		_primarySender.sendAppendFile(document, bytes, size, binary);
-			{
-				Sender sender = (Sender) _senders.get(j);
-				sender.sendAppendFile(document, bytes, size, binary);
-			}
-		}
 	}
 	
 
@@ -296,23 +291,18 @@ public class ServerUpdateHandler extends UpdateHandler
 			
 			_commandGenerator.generateResponse(document, _dataObjects);
 
-			synchronized (_senders){
+			_primarySender.sendDocument(document, 20);
-				for (int j = 0; j < _senders.size(); j++)
-				{
-					Sender sender = (Sender) _senders.get(j);
-					sender.sendDocument(document, 5);
 			if (_pendingKeepAliveConfirmation != null)
 			{
-						sender.sendKeepAliveConfirmation(_pendingKeepAliveConfirmation);
+				_primarySender.sendKeepAliveConfirmation(_pendingKeepAliveConfirmation);
 				_pendingKeepAliveConfirmation = null;
 			}
 			if (_pendingKeepAliveRequest != null)
 			{
-						sender.sendKeepAliveRequest(_pendingKeepAliveRequest);
+				_primarySender.sendKeepAliveRequest(_pendingKeepAliveRequest);
 				_pendingKeepAliveRequest = null;
 			}
-				}
+
-			}
 
 			for (int i = 0; i < _dataObjects.size(); i++)
 			{
@@ -364,6 +354,9 @@ public class ServerUpdateHandler extends UpdateHandler
 				removeSender(sender);
 			}
 		}	
+		if (_primarySender != null && _primarySender.socket() == socket){
+			_primarySender = null;
+		}
 	}
 	
 	/**
@@ -380,13 +373,7 @@ public class ServerUpdateHandler extends UpdateHandler
 		document.setParent(null);
 		//DataElement document = _dataStore.createObject(null, DataStoreResources.REQUEST_CLASS_TYPE, className);
 
-		synchronized (_senders){
+		_primarySender.requestClass(document);	
-			for (int j = 0; j < _senders.size(); j++)
-			{
-				Sender sender = (Sender) _senders.get(j);
-				sender.requestClass(document);
-			}
-		}	
 	}
 	
 
@@ -398,13 +385,8 @@ public class ServerUpdateHandler extends UpdateHandler
 		document.setPendingTransfer(true);
 		document.setParent(null);		
 
-		synchronized (_senders){
+		_primarySender.sendRemoteClassRunnable(document, runnable);	
-			for (int j = 0; j < _senders.size(); j++)
+
-			{
-				Sender sender = (Sender) _senders.get(j);
-				sender.sendRemoteClassRunnable(document, runnable);
-			}
-		}
 		notifyInput();
 	}
 
@@ -493,12 +475,6 @@ public class ServerUpdateHandler extends UpdateHandler
 	 */
 	public void setGenerateBuffer(boolean flag)
 	{
-		synchronized (_senders){
+		_primarySender.setGenerateBuffer(flag);
-			for (int i = 0; i < _senders.size(); i++)
-			{
-				Sender sender = (Sender)_senders.get(i);
-				sender.setGenerateBuffer(flag);
-			}
-		}
 	}
 }