[Regression] - Connection should not process messages when in closed state

[kitd]

Version

4.5.10

Context

There appears to be a regression of #3249 . When the connection is closed in the connection handler, the request continues to be dispatched to the request handlers.

Do you have a reproducer?

package com.example.starter;

import io.vertx.core.AbstractVerticle;
import io.vertx.core.Promise;
import io.vertx.core.Vertx;
import io.vertx.core.http.HttpServer;
import io.vertx.ext.web.Router;

public class MainVerticle extends AbstractVerticle {

    @Override
    public void start(Promise<Void> startPromise) throws Exception {
        HttpServer server = vertx.createHttpServer();

        Router router = Router.router(vertx);
        router.route()
                .handler(ctx -> {
                    System.out.println("!!!!!!!!!!!!! REQUEST HANDLER");
                    ctx.response()
                        .putHeader("content-type", "text/plain")
                        .end("Hello world");
                });

        server
                .connectionHandler(conn -> {
                    conn.close().onSuccess(v -> System.out.println("------ CONNECTION CLOSED -----"));
                })
                .requestHandler(router)
                .listen(8888).onComplete(http -> {
                    if (http.succeeded()) {
                        startPromise.complete();
                        System.out.println("HTTP server started on port 8888");
                    } else {
                        startPromise.fail(http.cause());
                    }
                });
    }

    public static void main(String[] args) {
        Vertx vertx = Vertx.vertx();
        vertx.deployVerticle(new MainVerticle());
    }
}

Steps to reproduce

1. Run the above code
2. Send a request to http://localhost:8888
3. See that even when the CONNECTION CLOSED message appears, the REQUEST HANDLER message also appears

Extra

Linux Fedora 40

java -version
openjdk version "17.0.9" 2023-10-17
IBM Semeru Runtime Open Edition 17.0.9.0 (build 17.0.9+9)
Eclipse OpenJ9 VM 17.0.9.0 (build openj9-0.41.0, JRE 17 Linux amd64-64-Bit Compressed References 20231017_614 (JIT enabled, AOT enabled)
OpenJ9   - 461bf3c70
OMR      - 5eee6ad9d
JCL      - 3699725139c based on jdk-17.0.9+9)

[kitd]

A further observation:

If you issue an HTTP2 request, it not just calls the request handler, but manages to return data to the client. This may be a timing issue, with the response being sent before the close() call has completed?

[kitd]

More observations:

When using ALPN thus:

        Buffer caPem = vertx.fileSystem().readFileBlocking("<cert-pem-file>");
        Buffer caKey = vertx.fileSystem().readFileBlocking("<cert-key-file>");

        PemKeyCertOptions pemOptions = new PemKeyCertOptions().
            setKeyValue(caKey).
            setCertValue(caPem);

        HttpServerOptions opts = new HttpServerOptions()
            .setSsl(true)
            .setUseAlpn(true)
            .setPemKeyCertOptions(pemOptions);

        HttpServer server = vertx.createHttpServer(opts);

it honours the closed connection and works as expected.

[kitd]

Here are my findings with various configurations of server using TLS & ALPN, and clients using HTTP1.1, HTTP1.1 upgrading to 2.0, and HTTP2.0 only. I hope this is useful:

HTTPS

	Client HTTP1.1 only	Client HTTP1.1 => HTTP2	Client HTTP2 only
Server with ALPN	Request handler not called	Request handler not called	Request handler not called
Server without ALPN	Request handler called with closed connection	Request handler called with closed connection	Request handler called with closed connection

Edit: the above results are when the client uses ALPN. With that disabled, results are variable, with the same parameters producing different results for different runs. I suspect there may be timing issues there.

HTTP

Client HTTP1.1 only	Client HTTP1.1 => HTTP2	Client HTTP2 only
Request handler called with closed connection	Request handler called with closed connection	Request handler called with open connection, data returned

[kitd]

FWIW, here's a test to test for the issue:

import java.util.concurrent.atomic.AtomicBoolean;

import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.vertx.core.Vertx;
import io.vertx.core.http.HttpClient;
import io.vertx.core.http.HttpClientResponse;
import io.vertx.core.http.HttpMethod;
import io.vertx.core.http.HttpServerOptions;
import io.vertx.ext.web.Router;
import io.vertx.ext.web.client.WebClientOptions;
import io.vertx.junit5.RunTestOnContext;
import io.vertx.junit5.VertxExtension;
import io.vertx.junit5.VertxTestContext;

@ExtendWith(VertxExtension.class)
public class ClosedConnectionTest {
    @RegisterExtension
    final static RunTestOnContext rtoc = new RunTestOnContext();
    static Vertx vertx;

    @BeforeAll
    static void prepare(VertxTestContext testContext) {
        vertx = rtoc.vertx();
        testContext.completeNow();
    }

    // Test whether issue https://github.com/eclipse-vertx/vert.x/issues/5340 has been fixed.
    @Test
    void testHandlerCalledWithClosedConnection(VertxTestContext testContext) {
        Router router = Router.router(vertx);
        AtomicBoolean handlerCalled = new AtomicBoolean(false);

        // If this handler gets called (when we have closed the connection further down)
        // then issue not resolved.
        router.route("/").handler(ctx -> {
            handlerCalled.compareAndExchange(false, true);
        });

        int port = 8080;

        vertx.createHttpServer()
            .connectionHandler(conn -> conn.close())
            .requestHandler(router)
            .listen(port).onComplete(testContext.succeeding())
            .compose(svr -> {
                HttpClient client = vertx.createHttpClient();
                return client.request(HttpMethod.GET, port, "localhost", "/");
            }).onComplete(testContext.succeeding())
            .compose(req -> req.send())
            .onComplete(
                // Connection is closed. No response should have been received 
                r -> testContext.failNow("There should be no response"),
                // On failed response, check whether request handler was called. 
                // If so, issue not resolved
                t -> {
                    if (handlerCalled.get()) {
                        testContext.completeNow();
                    } else {
                        testContext.failNow("Request handler was not called. Issue #5340 fixed");
                    }
                });
    }
}

[vietj]

@kitd thank I'll look at it