[jira] Commented: (AMQ-2820) Deadlock occur in the RegionBroker.java when remove stale connection

["Rob Davies (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/activemq/browse/AMQ-2820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=60567#action_60567 ] 

Rob Davies commented on AMQ-2820:
---------------------------------

The thread would already have the lock on clientIdSet before calling removeConnection()  - so not sure why you think there would be a deadlock - do you have a stacktrace ?

> Deadlock occur in the RegionBroker.java when remove stale connection
> --------------------------------------------------------------------
>
>                 Key: AMQ-2820
>                 URL: https://issues.apache.org/activemq/browse/AMQ-2820
>             Project: ActiveMQ
>          Issue Type: Bug
>          Components: Broker
>    Affects Versions: 5.0.0, 5.1.0, 5.2.0, 5.3.0, 5.3.1, 5.3.2
>            Reporter: Suriyan
>            Priority: Minor
>
> {code:title=RegionBroker.java|borderStyle=solid}
> 213 	 @Override
> 214 	public void addConnection(ConnectionContext context, ConnectionInfo info) throws Exception {
> 215 	String clientId = info.getClientId();
> 216 	if (clientId == null) {
> 217 	throw new InvalidClientIDException("No clientID specified for connection request");
> 218 	}
> 219 	synchronized (clientIdSet) {
> 220 	ConnectionContext oldContext = clientIdSet.get(clientId);
> 221 	if (oldContext != null) {
> 222 	if (context.isFaultTolerant() || context.isNetworkConnection()){
> 223 	//remove the old connection
> 224 	try{
> 225 	removeConnection(oldContext, info, new Exception("remove stale client"));
> 226 	}catch(Exception e){
> 227 	LOG.warn("Failed to remove stale connection ",e);
> 228 	}
> 229 	}else{
> 230 	throw new InvalidClientIDException("Broker: " + getBrokerName() + " - Client: " + clientId + " already connected from "
> 231 	+ oldContext.getConnection().getRemoteAddress());
> 232 	}
> 233 	} else {
> 234 	clientIdSet.put(clientId, context);
> 235 	}
> 236 	}
> 237 	
> 238 	connections.add(context.getConnection());
> 239 	}
> 240 	
> 241 	@Override
> 242 	public void removeConnection(ConnectionContext context, ConnectionInfo info, Throwable error) throws Exception {
> 243 	String clientId = info.getClientId();
> 244 	if (clientId == null) {
> 245 	throw new InvalidClientIDException("No clientID specified for connection disconnect request");
> 246 	}
> 247 	synchronized (clientIdSet) {
> 248 	ConnectionContext oldValue = clientIdSet.get(clientId);
> 249 	// we may be removing the duplicate connection, not the first
> 250 	// connection to be created
> 251 	// so lets check that their connection IDs are the same
> 252 	if (oldValue == context) {
> 253 	if (isEqual(oldValue.getConnectionId(), info.getConnectionId())) {
> 254 	clientIdSet.remove(clientId);
> 255 	}
> 256 	}
> 257 	}
> 258 	connections.remove(context.getConnection());
> 259 	} 
> {code}
> In the removeConnection(...) method need to synchronized for clientIdSet object. Deadlock will occur when removeConnection(...)  is called in the synchronized(clientIdSet) context from the addConnection(...) method (line 225).

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.