From fd9326a27b77a53978062538744468a052a9afb3 Mon Sep 17 00:00:00 2001 From: Dave McKnight Date: Mon, 14 Jul 2014 11:41:31 -0400 Subject: [PATCH] [439545][dstore] potential deadlock on senders during shutdown --- .../core/server/ConnectionEstablisher.java | 15 ++-- .../core/server/ServerUpdateHandler.java | 84 +++++++------------ 2 files changed, 39 insertions(+), 60 deletions(-) diff --git a/rse/plugins/org.eclipse.dstore.core/src/org/eclipse/dstore/core/server/ConnectionEstablisher.java b/rse/plugins/org.eclipse.dstore.core/src/org/eclipse/dstore/core/server/ConnectionEstablisher.java index b08c0715b90..b9f47f006cf 100644 --- a/rse/plugins/org.eclipse.dstore.core/src/org/eclipse/dstore/core/server/ConnectionEstablisher.java +++ b/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,11 +225,7 @@ 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$ } diff --git a/rse/plugins/org.eclipse.dstore.core/src/org/eclipse/dstore/internal/core/server/ServerUpdateHandler.java b/rse/plugins/org.eclipse.dstore.core/src/org/eclipse/dstore/internal/core/server/ServerUpdateHandler.java index 724751b2712..0932ff2081e 100644 --- a/rse/plugins/org.eclipse.dstore.core/src/org/eclipse/dstore/internal/core/server/ServerUpdateHandler.java +++ b/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(); @@ -213,14 +219,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++) - { - Sender sender = (Sender) _senders.get(j); - sender.sendFile(document, bytes, size, binary); - } - } + + _primarySender.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++) - { - Sender sender = (Sender) _senders.get(j); - sender.sendAppendFile(document, bytes, size, binary); - } - } + + _primarySender.sendAppendFile(document, bytes, size, binary); } @@ -296,23 +291,18 @@ public class ServerUpdateHandler extends UpdateHandler _commandGenerator.generateResponse(document, _dataObjects); - synchronized (_senders){ - for (int j = 0; j < _senders.size(); j++) - { - Sender sender = (Sender) _senders.get(j); - sender.sendDocument(document, 5); - if (_pendingKeepAliveConfirmation != null) - { - sender.sendKeepAliveConfirmation(_pendingKeepAliveConfirmation); - _pendingKeepAliveConfirmation = null; - } - if (_pendingKeepAliveRequest != null) - { - sender.sendKeepAliveRequest(_pendingKeepAliveRequest); - _pendingKeepAliveRequest = null; - } - } + _primarySender.sendDocument(document, 20); + if (_pendingKeepAliveConfirmation != null) + { + _primarySender.sendKeepAliveConfirmation(_pendingKeepAliveConfirmation); + _pendingKeepAliveConfirmation = null; } + if (_pendingKeepAliveRequest != null) + { + _primarySender.sendKeepAliveRequest(_pendingKeepAliveRequest); + _pendingKeepAliveRequest = null; + } + for (int i = 0; i < _dataObjects.size(); i++) { @@ -363,7 +353,10 @@ public class ServerUpdateHandler extends UpdateHandler sender.sendDocument(document, 2); 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){ - for (int j = 0; j < _senders.size(); j++) - { - Sender sender = (Sender) _senders.get(j); - sender.requestClass(document); - } - } + _primarySender.requestClass(document); } @@ -398,13 +385,8 @@ public class ServerUpdateHandler extends UpdateHandler document.setPendingTransfer(true); document.setParent(null); - synchronized (_senders){ - for (int j = 0; j < _senders.size(); j++) - { - Sender sender = (Sender) _senders.get(j); - sender.sendRemoteClassRunnable(document, runnable); - } - } + _primarySender.sendRemoteClassRunnable(document, runnable); + notifyInput(); } @@ -493,12 +475,6 @@ public class ServerUpdateHandler extends UpdateHandler */ public void setGenerateBuffer(boolean flag) { - synchronized (_senders){ - for (int i = 0; i < _senders.size(); i++) - { - Sender sender = (Sender)_senders.get(i); - sender.setGenerateBuffer(flag); - } - } + _primarySender.setGenerateBuffer(flag); } }