calling BlockingNettyContext.shutdown() does not clear handlers

[bclozel]

This issue is linked with spring-projects/spring-boot#9146
When trying to fix devtools support in Spring Boot (i.e. stopping the server, recreating a new Spring context and starting a new server - all that without stopping the JVM), I've found that disposing a server and starting a new one has not the expected behavior.

Here's a repro case that shows the behavior:

package com.example.demo;

import org.assertj.core.api.Assertions;
import org.junit.Test;
import reactor.ipc.netty.http.client.HttpClient;
import reactor.ipc.netty.http.client.HttpClientResponse;
import reactor.ipc.netty.http.server.HttpServer;
import reactor.ipc.netty.tcp.BlockingNettyContext;

import org.springframework.http.ResponseEntity;
import org.springframework.web.client.RestTemplate;

public class ReactorNettyRestart {

	@Test
	public void testRestart() {
		// start a first server with a handler that answers HTTP 200 OK
		BlockingNettyContext context = HttpServer.create(8080)
				.start((req, resp) -> resp.status(200).send());

		HttpClientResponse response = HttpClient.create(8080).get("/").block();

		// checking the response status, OK
		Assertions.assertThat(response.status().code()).isEqualTo(200);
		// dispose the Netty context and wait for the channel close
		context.shutdown();
		context.getContext().onClose().block();

		// create a totally new server instance, with a different handler that answers HTTP 201
		context = HttpServer.create(8080)
				.start((req, resp) -> resp.status(201).send());

		response = HttpClient.create(8080).get("/").block();

		// fails, response status is 200 and debugging shows the the previous handler is called
		Assertions.assertThat(response.status().code()).isEqualTo(201);
		context.shutdown();
		context.getContext().onClose().block();
	}

	@Test
	public void testRestartBlockingClient() {

		RestTemplate template = new RestTemplate();
		// start a first server with a handler that answers HTTP 200 OK
		BlockingNettyContext context = HttpServer.create(8080)
				.start((req, resp) -> resp.status(200).send());

		ResponseEntity<String> response = template.getForEntity("http://localhost:8080/", String.class);

		// checking the response status, OK
		Assertions.assertThat(response.getStatusCode().value()).isEqualTo(200);
		// dispose the Netty context and wait for the channel close
		context.shutdown();
		context.getContext().onClose().block();

		// create a totally new server instance, with a different handler that answers HTTP 201
		context = HttpServer.create(8080)
				.start((req, resp) -> resp.status(201).send());

		response = template.getForEntity("http://localhost:8080/", String.class);

		// fails, response status is 200 and debugging shows the the previous handler is called
		Assertions.assertThat(response.getStatusCode().value()).isEqualTo(201);
		context.shutdown();
		context.getContext().onClose().block();
	}

}

Am I misusing the HttpServer API?
Is there some state that I forgot to clear?

[smaldini]

The issue has more to do with the very currently limited lifecycle to be reworked.
Meanwhile, to fully reset the client pool, use HttpResources.reset().

[simonbasle]

I have added global resource cleanup as part of the BlockingNettyContext shutdown() method (that comes after start or automatically on JVM shutdown when using startAndAwait). See #135
Wdyt @bclozel @smaldini could that be a good resolution?

[bclozel]

I've updated the code sample in the original issue, and I can still reproduce the issue with the current 0.7.0 SNAPSHOT.
The new code sample shows that this can be reproduced as well with a RestTemplate client, backed by a basic JDK client (so nothing reactor related).

Note that calling HttpResources.reset(); right after waiting for the context to be closed fixes the problem in both cases.

[sdeleuze]

I faced the very same issue in Spring Fu (spring-projects-experimental/spring-fu@6be8b90) and I think that's a rather annoying one (it keeps the port open after closing the server) that deserves to be fixed before Spring Framework 5.1 / Spring Boot 2.1 release IMO.

[smaldini]

We should have the following fix in a next milestone/RC:
when disposableServer.dispose() is called globalResource will be scanned for associated client connections and cleaned accordingly.

[sdeleuze]

I reopen this issue, I still see it in Spring Fu. To reproduce clone https://github.com/spring-projects/spring-fu, remove HttpResources.reset() instances from https://github.com/spring-projects/spring-fu/blob/master/kofu/src/test/kotlin/org/springframework/boot/kofu/web/AbstractWebServerDslTests.kt, you will see test failures with client requesting the wrong server.

[violetagg]

@sdeleuze Can you test PR #449

[sdeleuze]

The issue still occurs with latest snapshot, if you want to debug that I have created a branch that reproduces the issue: https://github.com/spring-projects/spring-fu/tree/reactor-netty-http-resources. You can run for example JacksonDslTests it fails without HttpResources.reset() at the end of each test.

[sdeleuze]

As noticed by @violetagg, Californium-BUILD-SNAPSHOT was not using Reactor Netty snapshots. Now that it is the case, I have been able to verify that the issue is fixed. Thanks.