Skip to content

Commit 3bab909

Browse files
authored
Fix race in HTTP response shutdown handling (#105306) (#105327)
Similar to #97301, the fix in #105293 was still not quite correct: we could in principle shut down the transport after checking `isOpen()` but before sending the message. Applying the same fix as for the transport layer here.
1 parent 63db62d commit 3bab909

File tree

2 files changed

+12
-7
lines changed

2 files changed

+12
-7
lines changed

docs/changelog/105306.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 105306
2+
summary: Fix race in HTTP response shutdown handling
3+
area: Network
4+
type: bug
5+
issues: []

modules/transport-netty4/src/main/java/org/elasticsearch/http/netty4/Netty4HttpChannel.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@
1414
import org.elasticsearch.common.util.concurrent.ListenableFuture;
1515
import org.elasticsearch.http.HttpChannel;
1616
import org.elasticsearch.http.HttpResponse;
17+
import org.elasticsearch.transport.TransportException;
1718
import org.elasticsearch.transport.netty4.Netty4TcpChannel;
1819

1920
import java.net.InetSocketAddress;
2021
import java.net.SocketAddress;
21-
import java.nio.channels.ClosedChannelException;
2222

2323
public class Netty4HttpChannel implements HttpChannel {
2424

@@ -32,12 +32,12 @@ public class Netty4HttpChannel implements HttpChannel {
3232

3333
@Override
3434
public void sendResponse(HttpResponse response, ActionListener<Void> listener) {
35-
if (isOpen()) {
36-
channel.writeAndFlush(response, Netty4TcpChannel.addPromise(listener, channel));
37-
} else {
38-
// No need to dispatch to the event loop just to fail this listener; moreover the channel might be closed because the whole
39-
// node is shutting down, in which case the event loop might not exist any more so the channel promise cannot be completed.
40-
listener.onFailure(new ClosedChannelException());
35+
// We need to both guard against double resolving the listener and not resolving it in case of event loop shutdown so we need to
36+
// use #notifyOnce here until https://github.com/netty/netty/issues/8007 is resolved.
37+
var wrapped = ActionListener.notifyOnce(listener);
38+
channel.writeAndFlush(response, Netty4TcpChannel.addPromise(wrapped, channel));
39+
if (channel.eventLoop().isShutdown()) {
40+
wrapped.onFailure(new TransportException("Cannot send HTTP response, event loop is shutting down."));
4141
}
4242
}
4343

0 commit comments

Comments
 (0)