Qortal/altcoinj
1
0
Fork 1
mirror of https://github.com/Qortal/altcoinj.git synced 2025-11-02 05:27:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

ConnectionHandler: don't leak nioConnectionHandler lock on an exception path. Probably fixes #1009

Browse Source
This commit is contained in:
Mike Hearn
2015-07-02 18:44:55 +02:00
parent ce58d93afd
commit 25c4554760
1 changed files with 13 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
core/src/main/java/org/bitcoinj/net/ConnectionHandler.java
View File

@@ -37,6 +37,8 @@ import java.util.concurrent.locks.ReentrantLock;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
// TODO: The locking in all this class is horrible and not really necessary. We should just run all network stuff on one thread.
/**
* A simple NIO MessageWriteTarget which handles all the business logic of a connection (reading+writing bytes).
* Used only by the NioClient and NioServer classes
@@ -64,7 +66,7 @@ class ConnectionHandler implements MessageWriteTarget {
private Set<ConnectionHandler> connectedHandlers;
public ConnectionHandler(StreamParserFactory parserFactory, SelectionKey key) throws IOException {
this(parserFactory.getNewParser(((SocketChannel)key.channel()).socket().getInetAddress(), ((SocketChannel)key.channel()).socket().getPort()), key);
this(parserFactory.getNewParser(((SocketChannel) key.channel()).socket().getInetAddress(), ((SocketChannel) key.channel()).socket().getPort()), key);
if (parser == null)
throw new IOException("Parser factory.getNewParser returned null");
}
@@ -89,12 +91,10 @@ class ConnectionHandler implements MessageWriteTarget {
// parser.setWriteTarget which might have re-entered already. In this case we shouldn't add ourselves
// to the connectedHandlers set.
lock.lock();
boolean alreadyClosed = false;
try {
alreadyClosed = closeCalled;
this.connectedHandlers = connectedHandlers;
if (!alreadyClosed)
checkState(connectedHandlers.add(this));
if (!closeCalled)
checkState(this.connectedHandlers.add(this));
} finally {
lock.unlock();
}
@@ -135,6 +135,7 @@ class ConnectionHandler implements MessageWriteTarget {
@Override
public void writeBytes(byte[] message) throws IOException {
boolean andUnlock = true;
lock.lock();
try {
// Network buffers are not unlimited (and are often smaller than some messages we may wish to send), and
@@ -151,21 +152,26 @@ class ConnectionHandler implements MessageWriteTarget {
setWriteOps();
} catch (IOException e) {
lock.unlock();
andUnlock = false;
log.warn("Error writing message to connection, closing connection", e);
closeConnection();
throw e;
} catch (CancelledKeyException e) {
lock.unlock();
andUnlock = false;
log.warn("Error writing message to connection, closing connection", e);
closeConnection();
throw new IOException(e);
} finally {
if (andUnlock)
lock.unlock();
}
lock.unlock();
}
@Override
// May NOT be called with lock held
@Override
public void closeConnection() {
checkState(!lock.isHeldByCurrentThread());
try {
channel.close();
} catch (IOException e) {