Skip to content

Commit ebef1f8

Browse files
committed
[Java] More carefully ensure sockets are closed in NioPeerHandler
There is no indication that the sockets are leaking in the handling loop, but its good to be careful and ensure we always both close the SocketChannel and cancel the key registration. During connection, however, it appears we may leak a SocketChannel if the connection times out, leaking a file descriptor at least until the GC kicks in and cleans up after us. Here we are more careful.
1 parent 7a5639b commit ebef1f8

File tree

1 file changed

+20
-11
lines changed

1 file changed

+20
-11
lines changed

src/main/java/org/ldk/batteries/NioPeerHandler.java

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public long send_data(byte[] data, boolean resume_read) {
8787
(peer.key.interestOps() | SelectionKey.OP_READ) & (~SelectionKey.OP_WRITE)));
8888
}
8989
return written;
90-
} catch (IOException e) {
90+
} catch (IOException|CancelledKeyException ignored) {
9191
// Most likely the socket is disconnected, let the background thread handle it.
9292
return 0;
9393
}
@@ -97,7 +97,7 @@ public long send_data(byte[] data, boolean resume_read) {
9797
public void disconnect_socket() {
9898
try {
9999
do_selector_action(() -> {
100-
peer.key.cancel();
100+
try { peer.key.cancel(); } catch (CancelledKeyException ignored) {}
101101
peer.key.channel().close();
102102
});
103103
} catch (IOException ignored) { }
@@ -189,8 +189,8 @@ public NioPeerHandler(PeerManager manager) throws IOException {
189189
if (key.isValid() && (key.interestOps() & SelectionKey.OP_WRITE) != 0 && key.isWritable()) {
190190
Result_NonePeerHandleErrorZ res = this.peer_manager.write_buffer_space_avail(peer.descriptor);
191191
if (res instanceof Result_NonePeerHandleErrorZ.Result_NonePeerHandleErrorZ_Err) {
192-
key.channel().close();
193192
key.cancel();
193+
key.channel().close();
194194
}
195195
}
196196
if (key.isValid() && (key.interestOps() & SelectionKey.OP_READ) != 0 && key.isReadable()) {
@@ -199,6 +199,7 @@ public NioPeerHandler(PeerManager manager) throws IOException {
199199
if (read == -1) {
200200
this.peer_manager.socket_disconnected(peer.descriptor);
201201
key.cancel();
202+
key.channel().close(); // This may throw, we read -1 so the channel should already be closed, but do this to be safe
202203
} else if (read > 0) {
203204
((Buffer)buf).flip();
204205
// This code is quite hot during initial network graph sync, so we go a ways out of
@@ -223,15 +224,15 @@ public NioPeerHandler(PeerManager manager) throws IOException {
223224
key.interestOps(key.interestOps() & (~SelectionKey.OP_READ));
224225
}
225226
} else {
226-
key.channel().close();
227227
key.cancel();
228+
key.channel().close();
228229
}
229230
bindings.CResult_boolPeerHandleErrorZ_free(read_result_pointer);
230231
}
231232
}
232233
} catch (IOException ignored) {
233-
try { key.channel().close(); } catch (IOException ignored2) { }
234234
key.cancel();
235+
try { key.channel().close(); } catch (IOException ignored2) { }
235236
peer_manager.socket_disconnected(peer.descriptor);
236237
}
237238
} catch (CancelledKeyException e) {
@@ -265,13 +266,21 @@ private void dummy_check_return_type_matches_manual_memory_code_above(Peer peer)
265266
*/
266267
public void connect(byte[] their_node_id, SocketAddress remote, int timeout_ms) throws IOException {
267268
SocketChannel chan = SocketChannel.open();
268-
chan.configureBlocking(false);
269-
Selector open_selector = Selector.open();
270-
chan.register(open_selector, SelectionKey.OP_CONNECT);
271-
if (!chan.connect(remote)) {
272-
open_selector.select(timeout_ms);
269+
boolean connected;
270+
try {
271+
chan.configureBlocking(false);
272+
Selector open_selector = Selector.open();
273+
chan.register(open_selector, SelectionKey.OP_CONNECT);
274+
if (!chan.connect(remote)) {
275+
open_selector.select(timeout_ms);
276+
}
277+
connected = chan.finishConnect();
278+
} catch (IOException e) {
279+
try { chan.close(); } catch (IOException _e) { }
280+
throw e;
273281
}
274-
if (!chan.finishConnect()) { // Note that this may throw its own IOException if we failed for another reason
282+
if (!connected) {
283+
try { chan.close(); } catch (IOException _e) { }
275284
throw new IOException("Timed out");
276285
}
277286
Peer peer = setup_socket(chan);

0 commit comments

Comments
 (0)