Fix memory leak of HTTP server on bind failure

[SgtSilvio]

Fix issue #2843 by closing channel on bind (and other) exception

[pivotal-cla]

@SgtSilvio Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-cla]

@SgtSilvio Thank you for signing the Contributor License Agreement!

[violetagg]

Thanks for the PR!

Can you provide a test?

[SgtSilvio]

Can you provide a test?

Good point.
The original issue with the memory leak on bind failure is pretty hard to test, as the TransportConnector.bind method does not return a handle of the channel in that case. Maybe we can get a handle by overriding TransportConfig.connectionFactory in the test.

[violetagg]

May be we can limit the test to checking channel is opened or not?

[violetagg]

May be something like this below:

List<Channel> channels = new ArrayList<>();
HttpServer.create().doOnChannelInit((obs, ch, addr) -> channels.add(ch))....;
// Inspect channel.isOpen()

[violetagg]

May be something like this below:

List<Channel> channels = new ArrayList<>();
HttpServer.create().doOnChannelInit((obs, ch, addr) -> channels.add(ch))....;
// Inspect channel.isOpen()

Do you think this will help for the test?

[SgtSilvio]

I will still provide a test, but doOnChannelInit does not work unfortunately as this is only called for child channels and not for the server channel.

[violetagg]

I will still provide a test, but doOnChannelInit does not work unfortunately as this is only called for child channels and not for the server channel.

ouch correct ...

Then try directly to test TransportConnector.bind and just mock the TransportConfig.

Something like

SocketAddress socketAddress = new InetSocketAddress("localhost", 12345);
List<Channel> channels = new ArrayList<>();
ChannelInitializer<Channel> channelInitializer = new ChannelInitializer<Channel>() {
	@Override
	protected void initChannel(Channel ch) {
		channels.add(ch);
	}
};
TransportConnector.bind(mockConfig, channelInitializer, socketAddress, false)

[SgtSilvio]

Thanks, now added a test that fails without and succeeds with the changes.

[violetagg]

Can you update also TransportConnector copyright end year?

> The following files had format violations:
      reactor-netty-core/src/main/java/reactor/netty/transport/TransportConnector.java
          @@ -1,5 +1,5 @@
           /*
          -·*·Copyright·(c)·2020-2022·VMware,·Inc.·or·its·affiliates,·All·Rights·Reserved.
          +·*·Copyright·(c)·2020-2023·VMware,·Inc.·or·its·affiliates,·All·Rights·Reserved.
           ·*
           ·*·Licensed·under·the·Apache·License,·Version·2.0·(the·"License");
           ·*·you·may·not·use·this·file·except·in·compliance·with·the·License.
  Run './gradlew :spotlessApply' to fix these violations.

[violetagg]

@reactor/netty-team PTAL

[pderop]

@SgtSilvio , nice !

+1

[SgtSilvio]

Thanks for the very quick handling!