From 42b15f4f68fd3a3857ff279d32cc2e43c997a89f Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Tue, 13 Feb 2024 14:02:13 +0530 Subject: [PATCH 01/10] [Automated] Update the native jar versions --- ballerina/Ballerina.toml | 6 +++--- ballerina/CompilerPlugin.toml | 2 +- ballerina/Dependencies.toml | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ballerina/Ballerina.toml b/ballerina/Ballerina.toml index 0123d599a9..aa63c7cdb4 100644 --- a/ballerina/Ballerina.toml +++ b/ballerina/Ballerina.toml @@ -1,7 +1,7 @@ [package] org = "ballerina" name = "http" -version = "2.10.6" +version = "2.10.7" authors = ["Ballerina"] keywords = ["http", "network", "service", "listener", "client"] repository = "https://github.com/ballerina-platform/module-ballerina-http" @@ -16,8 +16,8 @@ graalvmCompatible = true [[platform.java17.dependency]] groupId = "io.ballerina.stdlib" artifactId = "http-native" -version = "2.10.6" -path = "../native/build/libs/http-native-2.10.6.jar" +version = "2.10.7" +path = "../native/build/libs/http-native-2.10.7-SNAPSHOT.jar" [[platform.java17.dependency]] groupId = "io.ballerina.stdlib" diff --git a/ballerina/CompilerPlugin.toml b/ballerina/CompilerPlugin.toml index e050f85ce0..e4d4ba0169 100644 --- a/ballerina/CompilerPlugin.toml +++ b/ballerina/CompilerPlugin.toml @@ -3,4 +3,4 @@ id = "http-compiler-plugin" class = "io.ballerina.stdlib.http.compiler.HttpCompilerPlugin" [[dependency]] -path = "../compiler-plugin/build/libs/http-compiler-plugin-2.10.6.jar" +path = "../compiler-plugin/build/libs/http-compiler-plugin-2.10.7-SNAPSHOT.jar" diff --git a/ballerina/Dependencies.toml b/ballerina/Dependencies.toml index 321c5e85c0..f874d3df38 100644 --- a/ballerina/Dependencies.toml +++ b/ballerina/Dependencies.toml @@ -25,7 +25,7 @@ modules = [ [[package]] org = "ballerina" name = "cache" -version = "3.7.0" +version = "3.7.1" dependencies = [ {org = "ballerina", name = "constraint"}, {org = "ballerina", name = "jballerina.java"}, @@ -76,7 +76,7 @@ modules = [ [[package]] org = "ballerina" name = "http" -version = "2.10.6" +version = "2.10.7" dependencies = [ {org = "ballerina", name = "auth"}, {org = "ballerina", name = "cache"}, @@ -283,7 +283,7 @@ modules = [ [[package]] org = "ballerina" name = "observe" -version = "1.2.0" +version = "1.2.2" dependencies = [ {org = "ballerina", name = "jballerina.java"} ] From cbbca0b5988afe3e3053c058570fbfa537d174d4 Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Tue, 13 Feb 2024 17:48:18 +0530 Subject: [PATCH 02/10] Update the promise provided for connection close --- .../contractimpl/sender/http2/Http2ClientChannel.java | 3 +++ .../contractimpl/sender/http2/Http2ConnectionManager.java | 4 +--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ClientChannel.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ClientChannel.java index ee3d0b0dbe..f11c6e506d 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ClientChannel.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ClientChannel.java @@ -303,6 +303,9 @@ public void onStreamClosed(Http2Stream stream) { @Override public void onGoAwayReceived(int lastStreamId, long errorCode, ByteBuf debugData) { + if (isStale.get()) { + return; + } markAsStale(); http2ClientChannel.inFlightMessages.forEach((streamId, outboundMsgHolder) -> { if (streamId > lastStreamId) { diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java index 9ea1d96166..dbfe5c5689 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java @@ -21,8 +21,6 @@ import io.ballerina.stdlib.http.transport.contractimpl.common.HttpRoute; import io.ballerina.stdlib.http.transport.contractimpl.common.states.Http2MessageStateContext; import io.ballerina.stdlib.http.transport.contractimpl.sender.channel.pool.PoolConfiguration; -import io.netty.channel.DefaultEventLoop; -import io.netty.util.concurrent.DefaultPromise; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -196,7 +194,7 @@ public void closeChannelAndEvict(Http2ClientChannel http2ClientChannel) { logger.warn("Specified channel does not exist in the stale list."); } http2ClientChannel.getConnection() - .close(new DefaultPromise(new DefaultEventLoop())); + .close(http2ClientChannel.getChannel().newPromise()); } }; timer.schedule(timerTask, poolConfiguration.getTimeBetweenStaleEviction(), From 1a8268732e046d6ea616448721decd42739a8588 Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Tue, 13 Feb 2024 21:52:43 +0530 Subject: [PATCH 03/10] Remove channel from stale pool --- .../sender/http2/Http2ClientChannel.java | 4 + .../sender/http2/Http2ConnectionManager.java | 9 +- .../sender/http2/Http2TargetHandler.java | 1 + ...ctionAfterTcpServerGoAwayScenarioTest.java | 139 ++++++++++++++++++ ...ctionAfterTcpServerGoAwayScenarioTest.java | 139 ++++++++++++++++++ ...ctionAfterTcpServerGoAwayScenarioTest.java | 1 - native/src/test/resources/testng.xml | 2 + 7 files changed, 292 insertions(+), 3 deletions(-) create mode 100644 native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseAfterConnectionEvictionAfterTcpServerGoAwayScenarioTest.java create mode 100644 native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseBeforeConnectionEvictionAfterTcpServerGoAwayScenarioTest.java diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ClientChannel.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ClientChannel.java index f11c6e506d..6f069e5742 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ClientChannel.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ClientChannel.java @@ -334,6 +334,10 @@ void markAsStale() { } } + void removeClosedChannelFromStalePool() { + http2ConnectionManager.removeClosedChannelFromStalePool(this); + } + boolean hasInFlightMessages() { return !inFlightMessages.isEmpty(); } diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java index dbfe5c5689..9d4ad73083 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java @@ -21,6 +21,8 @@ import io.ballerina.stdlib.http.transport.contractimpl.common.HttpRoute; import io.ballerina.stdlib.http.transport.contractimpl.common.states.Http2MessageStateContext; import io.ballerina.stdlib.http.transport.contractimpl.sender.channel.pool.PoolConfiguration; +import io.netty.channel.DefaultEventLoop; +import io.netty.util.concurrent.DefaultPromise; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -163,6 +165,10 @@ void markClientChannelAsStale(HttpRoute httpRoute, Http2ClientChannel http2Clien http2StaleClientChannels.add(http2ClientChannel); } + void removeClosedChannelFromStalePool(Http2ClientChannel http2ClientChannel) { + http2StaleClientChannels.remove(http2ClientChannel); + } + private void initiateConnectionEvictionTask() { Timer timer = new Timer(true); TimerTask timerTask = new TimerTask() { @@ -193,8 +199,7 @@ public void closeChannelAndEvict(Http2ClientChannel http2ClientChannel) { if (!result) { logger.warn("Specified channel does not exist in the stale list."); } - http2ClientChannel.getConnection() - .close(http2ClientChannel.getChannel().newPromise()); + http2ClientChannel.getConnection().close(new DefaultPromise(new DefaultEventLoop())); } }; timer.schedule(timerTask, poolConfiguration.getTimeBetweenStaleEviction(), diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2TargetHandler.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2TargetHandler.java index b1031cc23c..aeb84762a6 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2TargetHandler.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2TargetHandler.java @@ -295,6 +295,7 @@ public void channelInactive(ChannelHandlerContext ctx) { LOG.debug("Channel is inactive"); } http2ClientChannel.destroy(); + http2ClientChannel.removeClosedChannelFromStalePool(); } private boolean isUnexpected100ContinueResponse(Http2Headers http2Headers, HttpCarbonMessage inboundReq) { diff --git a/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseAfterConnectionEvictionAfterTcpServerGoAwayScenarioTest.java b/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseAfterConnectionEvictionAfterTcpServerGoAwayScenarioTest.java new file mode 100644 index 0000000000..10b6ed521e --- /dev/null +++ b/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseAfterConnectionEvictionAfterTcpServerGoAwayScenarioTest.java @@ -0,0 +1,139 @@ +/* + * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.com) All Rights Reserved. + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.http.transport.http2.frameleveltests; + +import io.ballerina.stdlib.http.transport.contract.Constants; +import io.ballerina.stdlib.http.transport.contract.HttpClientConnector; +import io.ballerina.stdlib.http.transport.contract.HttpWsConnectorFactory; +import io.ballerina.stdlib.http.transport.contract.config.SenderConfiguration; +import io.ballerina.stdlib.http.transport.contract.config.TransportsConfiguration; +import io.ballerina.stdlib.http.transport.contractimpl.DefaultHttpWsConnectorFactory; +import io.ballerina.stdlib.http.transport.contractimpl.sender.channel.pool.PoolConfiguration; +import io.ballerina.stdlib.http.transport.message.HttpConnectorUtil; +import io.ballerina.stdlib.http.transport.util.DefaultHttpConnectorListener; +import io.ballerina.stdlib.http.transport.util.TestUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.annotations.AfterClass; +import org.testng.annotations.Test; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.ServerSocket; +import java.net.Socket; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import static io.ballerina.stdlib.http.transport.contract.Constants.REMOTE_SERVER_CLOSED_WHILE_READING_INBOUND_RESPONSE_BODY; +import static io.ballerina.stdlib.http.transport.http2.frameleveltests.FrameLevelTestUtils.GO_AWAY_FRAME_MAX_STREAM_05; +import static io.ballerina.stdlib.http.transport.http2.frameleveltests.FrameLevelTestUtils.HEADER_FRAME_STREAM_03; +import static io.ballerina.stdlib.http.transport.http2.frameleveltests.FrameLevelTestUtils.SETTINGS_FRAME; +import static io.ballerina.stdlib.http.transport.http2.frameleveltests.FrameLevelTestUtils.SETTINGS_FRAME_WITH_ACK; +import static io.ballerina.stdlib.http.transport.http2.frameleveltests.FrameLevelTestUtils.SLEEP_TIME; +import static io.ballerina.stdlib.http.transport.util.TestUtil.getDecoderErrorMessage; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.fail; + +/** + * This contains a test case where the tcp server sends a GoAway and the connection gets closed from the + * server side after an eviction occurs. The successfulness of this test case cannot be confirmed by the assert + * completely. We have to look at the logs in order to check whether there are any internal netty exceptions + */ +public class Http2ChannelCloseAfterConnectionEvictionAfterTcpServerGoAwayScenarioTest { + + private static final Logger LOGGER = + LoggerFactory.getLogger(Http2ChannelCloseAfterConnectionEvictionAfterTcpServerGoAwayScenarioTest.class); + private HttpClientConnector h2ClientWithPriorKnowledge; + private ServerSocket serverSocket; + + public HttpClientConnector setupHttp2PriorKnowledgeClient(long minIdleTimeInStaleState, + long timeBetweenStaleEviction) { + HttpWsConnectorFactory connectorFactory = new DefaultHttpWsConnectorFactory(); + PoolConfiguration poolConfiguration = new PoolConfiguration(); + poolConfiguration.setMinIdleTimeInStaleState(minIdleTimeInStaleState); + poolConfiguration.setTimeBetweenStaleEviction(timeBetweenStaleEviction); + TransportsConfiguration transportsConfiguration = new TransportsConfiguration(); + SenderConfiguration senderConfiguration = new SenderConfiguration(); + senderConfiguration.setPoolConfiguration(poolConfiguration); + senderConfiguration.setScheme(Constants.HTTP_SCHEME); + senderConfiguration.setHttpVersion(Constants.HTTP_2_0); + senderConfiguration.setForceHttp2(true); + return connectorFactory.createHttpClientConnector( + HttpConnectorUtil.getTransportProperties(transportsConfiguration), senderConfiguration); + } + + @Test + private void testChannelCloseAfterConnectionEvictionScenario() { + try { + runTcpServer(TestUtil.HTTP_SERVER_PORT); + h2ClientWithPriorKnowledge = setupHttp2PriorKnowledgeClient(3000, 1000); + CountDownLatch latch1 = new CountDownLatch(1); + DefaultHttpConnectorListener msgListener1 = TestUtil.sendRequestAsync(latch1, h2ClientWithPriorKnowledge); + latch1.await(TestUtil.HTTP2_RESPONSE_TIME_OUT, TimeUnit.SECONDS); + // Waiting more than the minIdleTimeInStaleState to trigger a connectionEviction try and a channelInactive + Thread.sleep(8000); + + String errorMsg1 = getDecoderErrorMessage(msgListener1); + assertEquals(errorMsg1, REMOTE_SERVER_CLOSED_WHILE_READING_INBOUND_RESPONSE_BODY); + } catch (InterruptedException | IOException e) { + LOGGER.error("Exception occurred"); + fail(); + } + } + + private void runTcpServer(int port) throws IOException { + if (serverSocket != null) { + serverSocket.close(); + } + new Thread(() -> { + try { + serverSocket = new ServerSocket(port); + LOGGER.info("HTTP/2 TCP Server listening on port " + port); + Socket clientSocket = serverSocket.accept(); + LOGGER.info("Accepted connection from: " + clientSocket.getInetAddress()); + try (OutputStream outputStream = clientSocket.getOutputStream()) { + sendGoAwayForASingleStream(outputStream); + } catch (Exception e) { + LOGGER.error(e.getMessage()); + } + } catch (IOException e) { + LOGGER.error(e.getMessage()); + } + }).start(); + } + + private void sendGoAwayForASingleStream(OutputStream outputStream) throws IOException, InterruptedException { + outputStream.write(SETTINGS_FRAME); + outputStream.write(SETTINGS_FRAME_WITH_ACK); + Thread.sleep(SLEEP_TIME); + outputStream.write(HEADER_FRAME_STREAM_03); + Thread.sleep(SLEEP_TIME); + // This will move the connection to the stale connections list + outputStream.write(GO_AWAY_FRAME_MAX_STREAM_05); + // Waiting more than the time of `minIdleTimeInStaleState` and exit the socket write to trigger a + // `channelInactive` after minIdleTime exceeds. + Thread.sleep(5000); + } + + @AfterClass + public void cleanUp() throws IOException { + h2ClientWithPriorKnowledge.close(); + serverSocket.close(); + } +} diff --git a/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseBeforeConnectionEvictionAfterTcpServerGoAwayScenarioTest.java b/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseBeforeConnectionEvictionAfterTcpServerGoAwayScenarioTest.java new file mode 100644 index 0000000000..0938418f89 --- /dev/null +++ b/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseBeforeConnectionEvictionAfterTcpServerGoAwayScenarioTest.java @@ -0,0 +1,139 @@ +/* + * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.com) All Rights Reserved. + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.http.transport.http2.frameleveltests; + +import io.ballerina.stdlib.http.transport.contract.Constants; +import io.ballerina.stdlib.http.transport.contract.HttpClientConnector; +import io.ballerina.stdlib.http.transport.contract.HttpWsConnectorFactory; +import io.ballerina.stdlib.http.transport.contract.config.SenderConfiguration; +import io.ballerina.stdlib.http.transport.contract.config.TransportsConfiguration; +import io.ballerina.stdlib.http.transport.contractimpl.DefaultHttpWsConnectorFactory; +import io.ballerina.stdlib.http.transport.contractimpl.sender.channel.pool.PoolConfiguration; +import io.ballerina.stdlib.http.transport.message.HttpConnectorUtil; +import io.ballerina.stdlib.http.transport.util.DefaultHttpConnectorListener; +import io.ballerina.stdlib.http.transport.util.TestUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testng.annotations.AfterClass; +import org.testng.annotations.Test; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.ServerSocket; +import java.net.Socket; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import static io.ballerina.stdlib.http.transport.contract.Constants.REMOTE_SERVER_CLOSED_WHILE_READING_INBOUND_RESPONSE_BODY; +import static io.ballerina.stdlib.http.transport.http2.frameleveltests.FrameLevelTestUtils.GO_AWAY_FRAME_MAX_STREAM_05; +import static io.ballerina.stdlib.http.transport.http2.frameleveltests.FrameLevelTestUtils.HEADER_FRAME_STREAM_03; +import static io.ballerina.stdlib.http.transport.http2.frameleveltests.FrameLevelTestUtils.SETTINGS_FRAME; +import static io.ballerina.stdlib.http.transport.http2.frameleveltests.FrameLevelTestUtils.SETTINGS_FRAME_WITH_ACK; +import static io.ballerina.stdlib.http.transport.http2.frameleveltests.FrameLevelTestUtils.SLEEP_TIME; +import static io.ballerina.stdlib.http.transport.util.TestUtil.getDecoderErrorMessage; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.fail; + +/** + * This contains a test case where the tcp server sends a GoAway and the connection gets closed from the + * server side before an eviction occurs. The successfulness of this test case cannot be confirmed by the assert + * completely. We have to look at the logs in order to check whether there are any internal netty exceptions + */ +public class Http2ChannelCloseBeforeConnectionEvictionAfterTcpServerGoAwayScenarioTest { + + private static final Logger LOGGER = + LoggerFactory.getLogger(Http2ChannelCloseBeforeConnectionEvictionAfterTcpServerGoAwayScenarioTest.class); + private HttpClientConnector h2ClientWithPriorKnowledge; + private ServerSocket serverSocket; + + public HttpClientConnector setupHttp2PriorKnowledgeClient(long minIdleTimeInStaleState, + long timeBetweenStaleEviction) { + HttpWsConnectorFactory connectorFactory = new DefaultHttpWsConnectorFactory(); + PoolConfiguration poolConfiguration = new PoolConfiguration(); + poolConfiguration.setMinIdleTimeInStaleState(minIdleTimeInStaleState); + poolConfiguration.setTimeBetweenStaleEviction(timeBetweenStaleEviction); + TransportsConfiguration transportsConfiguration = new TransportsConfiguration(); + SenderConfiguration senderConfiguration = new SenderConfiguration(); + senderConfiguration.setPoolConfiguration(poolConfiguration); + senderConfiguration.setScheme(Constants.HTTP_SCHEME); + senderConfiguration.setHttpVersion(Constants.HTTP_2_0); + senderConfiguration.setForceHttp2(true); + return connectorFactory.createHttpClientConnector( + HttpConnectorUtil.getTransportProperties(transportsConfiguration), senderConfiguration); + } + + @Test + private void testChannelCloseBeforeConnectionEvictionScenario() { + try { + runTcpServer(TestUtil.HTTP_SERVER_PORT); + h2ClientWithPriorKnowledge = setupHttp2PriorKnowledgeClient(5000, 1000); + CountDownLatch latch1 = new CountDownLatch(1); + DefaultHttpConnectorListener msgListener1 = TestUtil.sendRequestAsync(latch1, h2ClientWithPriorKnowledge); + latch1.await(TestUtil.HTTP2_RESPONSE_TIME_OUT, TimeUnit.SECONDS); + // Waiting more than the minIdleTimeInStaleState to trigger a connectionEviction try + Thread.sleep(8000); + + String errorMsg1 = getDecoderErrorMessage(msgListener1); + assertEquals(errorMsg1, REMOTE_SERVER_CLOSED_WHILE_READING_INBOUND_RESPONSE_BODY); + } catch (InterruptedException | IOException e) { + LOGGER.error("Exception occurred"); + fail(); + } + } + + private void runTcpServer(int port) throws IOException { + if (serverSocket != null) { + serverSocket.close(); + } + new Thread(() -> { + try { + serverSocket = new ServerSocket(port); + LOGGER.info("HTTP/2 TCP Server listening on port " + port); + Socket clientSocket = serverSocket.accept(); + LOGGER.info("Accepted connection from: " + clientSocket.getInetAddress()); + try (OutputStream outputStream = clientSocket.getOutputStream()) { + sendGoAwayForASingleStream(outputStream); + } catch (Exception e) { + LOGGER.error(e.getMessage()); + } + } catch (IOException e) { + LOGGER.error(e.getMessage()); + } + }).start(); + } + + private void sendGoAwayForASingleStream(OutputStream outputStream) throws IOException, InterruptedException { + outputStream.write(SETTINGS_FRAME); + outputStream.write(SETTINGS_FRAME_WITH_ACK); + Thread.sleep(SLEEP_TIME); + outputStream.write(HEADER_FRAME_STREAM_03); + Thread.sleep(SLEEP_TIME); + // This will move the connection to the stale connections list + outputStream.write(GO_AWAY_FRAME_MAX_STREAM_05); + // Waiting less than the time of `minIdleTimeInStaleState` and exit the socket write to trigger a + // `channelInactive` before minIdleTime exceeds. + Thread.sleep(2000); + } + + @AfterClass + public void cleanUp() throws IOException { + h2ClientWithPriorKnowledge.close(); + serverSocket.close(); + } +} diff --git a/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ConnectionEvictionAfterTcpServerGoAwayScenarioTest.java b/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ConnectionEvictionAfterTcpServerGoAwayScenarioTest.java index a2a1ef1416..1bfc330f51 100644 --- a/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ConnectionEvictionAfterTcpServerGoAwayScenarioTest.java +++ b/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ConnectionEvictionAfterTcpServerGoAwayScenarioTest.java @@ -129,7 +129,6 @@ private void testConnectionEvictionAfterAllStreamsAreClosedScenario() { private void testConnectionEvictionBeforeAllStreamsAreClosedScenario() { try { runTcpServer(TestUtil.HTTP_SERVER_PORT); - // Setting to -1 will make the runner to wait until all pending streams are completed h2ClientWithPriorKnowledge = setupHttp2PriorKnowledgeClient(5000, 1000); CountDownLatch latch1 = new CountDownLatch(2); DefaultHttpConnectorListener msgListener1 = TestUtil.sendRequestAsync(latch1, h2ClientWithPriorKnowledge); diff --git a/native/src/test/resources/testng.xml b/native/src/test/resources/testng.xml index 3b35eae3f3..1dba82ce7d 100644 --- a/native/src/test/resources/testng.xml +++ b/native/src/test/resources/testng.xml @@ -167,6 +167,8 @@ + + From 4f38a70b35843dcee0d4a50203bb9a3cb716f4df Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Tue, 13 Feb 2024 22:02:29 +0530 Subject: [PATCH 04/10] Update changelog.md --- changelog.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changelog.md b/changelog.md index b85ce2f1e8..ad5cd700b5 100644 --- a/changelog.md +++ b/changelog.md @@ -5,6 +5,11 @@ This file contains all the notable changes done to the Ballerina HTTP package th The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed +- [Fix connection getting closed by stale eviction task after it has been closed by the server](https://github.com/ballerina-platform/ballerina-library/issues/6050) + ## [2.10.6] - 2024-02-01 ### Added From 5cd88f29184567617cf1050ed25e419a69cb3afc Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Tue, 13 Feb 2024 22:17:49 +0530 Subject: [PATCH 05/10] Suppress warnings --- .../contractimpl/sender/http2/Http2ConnectionManager.java | 1 + 1 file changed, 1 insertion(+) diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java index 9d4ad73083..48a70c085d 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java @@ -165,6 +165,7 @@ void markClientChannelAsStale(HttpRoute httpRoute, Http2ClientChannel http2Clien http2StaleClientChannels.add(http2ClientChannel); } + @SuppressWarnings("java:S899") void removeClosedChannelFromStalePool(Http2ClientChannel http2ClientChannel) { http2StaleClientChannels.remove(http2ClientChannel); } From f64c49394aa909a5b53015bd51bcc83ee7a195ba Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Wed, 14 Feb 2024 08:46:01 +0530 Subject: [PATCH 06/10] Fix review comments --- .../sender/http2/Http2ConnectionManager.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java index 48a70c085d..ca74ce2a16 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java @@ -36,6 +36,8 @@ */ public class Http2ConnectionManager { + Logger logger = LoggerFactory.getLogger(this.getClass()); + private final Http2ChannelPool http2ChannelPool = new Http2ChannelPool(); private final BlockingQueue http2StaleClientChannels = new LinkedBlockingQueue<>(); private final PoolConfiguration poolConfiguration; @@ -165,15 +167,15 @@ void markClientChannelAsStale(HttpRoute httpRoute, Http2ClientChannel http2Clien http2StaleClientChannels.add(http2ClientChannel); } - @SuppressWarnings("java:S899") void removeClosedChannelFromStalePool(Http2ClientChannel http2ClientChannel) { - http2StaleClientChannels.remove(http2ClientChannel); + if (!http2StaleClientChannels.remove(http2ClientChannel)) { + logger.warn("Specified channel does not exist in the stale list."); + } } private void initiateConnectionEvictionTask() { Timer timer = new Timer(true); TimerTask timerTask = new TimerTask() { - Logger logger = LoggerFactory.getLogger(this.getClass()); @Override public void run() { http2StaleClientChannels.forEach(http2ClientChannel -> { @@ -196,11 +198,8 @@ public void run() { } public void closeChannelAndEvict(Http2ClientChannel http2ClientChannel) { - boolean result = http2StaleClientChannels.remove(http2ClientChannel); - if (!result) { - logger.warn("Specified channel does not exist in the stale list."); - } - http2ClientChannel.getConnection().close(new DefaultPromise(new DefaultEventLoop())); + removeClosedChannelFromStalePool(http2ClientChannel); + http2ClientChannel.getConnection().close(http2ClientChannel.getChannel().newPromise()); } }; timer.schedule(timerTask, poolConfiguration.getTimeBetweenStaleEviction(), From ae418dcddb2015ba10994c2f6f1d879bf1f08e44 Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Wed, 14 Feb 2024 08:46:14 +0530 Subject: [PATCH 07/10] Disable intermittently failing testcase --- .../tests/http2_configuration_request_limit_config_test.bal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal b/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal index 92fe98ebed..d7a71215da 100644 --- a/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal +++ b/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal @@ -145,7 +145,7 @@ function testHttp2ValidHeaderLength() returns error? { } //Tests the behaviour when header size is greater than the configured threshold -@test:Config {} +@test:Config {enable: false} function testHttp2InvalidHeaderLength() returns error? { http:Client limitClient = check new ("http://localhost:" + http2RequestLimitsTestPort3.toString(), http2Settings = {http2PriorKnowledge: true}); From 1102b477cc0301bfc530d33dc0dd6e88a84a6fa4 Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Wed, 14 Feb 2024 08:48:58 +0530 Subject: [PATCH 08/10] Remove unused imports --- .../contractimpl/sender/http2/Http2ConnectionManager.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java index ca74ce2a16..a0e4e1a5e8 100644 --- a/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java +++ b/native/src/main/java/io/ballerina/stdlib/http/transport/contractimpl/sender/http2/Http2ConnectionManager.java @@ -21,8 +21,6 @@ import io.ballerina.stdlib.http.transport.contractimpl.common.HttpRoute; import io.ballerina.stdlib.http.transport.contractimpl.common.states.Http2MessageStateContext; import io.ballerina.stdlib.http.transport.contractimpl.sender.channel.pool.PoolConfiguration; -import io.netty.channel.DefaultEventLoop; -import io.netty.util.concurrent.DefaultPromise; import org.slf4j.Logger; import org.slf4j.LoggerFactory; From 820a1006c19cba668bbbd1d4ac9acb0c5cdc1ee5 Mon Sep 17 00:00:00 2001 From: dilanSachi Date: Wed, 14 Feb 2024 08:58:17 +0530 Subject: [PATCH 09/10] Add issue to test case --- .../tests/http2_configuration_request_limit_config_test.bal | 1 + 1 file changed, 1 insertion(+) diff --git a/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal b/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal index d7a71215da..a783173f72 100644 --- a/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal +++ b/ballerina-tests/http2-tests/tests/http2_configuration_request_limit_config_test.bal @@ -145,6 +145,7 @@ function testHttp2ValidHeaderLength() returns error? { } //Tests the behaviour when header size is greater than the configured threshold +// TODO: Enable after fixing this issue : https://github.com/ballerina-platform/ballerina-library/issues/4461 @test:Config {enable: false} function testHttp2InvalidHeaderLength() returns error? { http:Client limitClient = check new ("http://localhost:" + http2RequestLimitsTestPort3.toString(), From 52d283dccdb2e754a012db16648935df6f9452c0 Mon Sep 17 00:00:00 2001 From: Dilan Sachintha Nayanajith Date: Wed, 14 Feb 2024 09:18:27 +0530 Subject: [PATCH 10/10] Apply suggestions from code review Co-authored-by: Ayesh Almeida <77491511+ayeshLK@users.noreply.github.com> --- ...AfterConnectionEvictionAfterTcpServerGoAwayScenarioTest.java | 2 +- ...eforeConnectionEvictionAfterTcpServerGoAwayScenarioTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseAfterConnectionEvictionAfterTcpServerGoAwayScenarioTest.java b/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseAfterConnectionEvictionAfterTcpServerGoAwayScenarioTest.java index 10b6ed521e..0d9c6f28ee 100644 --- a/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseAfterConnectionEvictionAfterTcpServerGoAwayScenarioTest.java +++ b/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseAfterConnectionEvictionAfterTcpServerGoAwayScenarioTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.com) All Rights Reserved. + * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.com). * * WSO2 LLC. licenses this file to you under the Apache License, * Version 2.0 (the "License"); you may not use this file except diff --git a/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseBeforeConnectionEvictionAfterTcpServerGoAwayScenarioTest.java b/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseBeforeConnectionEvictionAfterTcpServerGoAwayScenarioTest.java index 0938418f89..b4f4ffbc72 100644 --- a/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseBeforeConnectionEvictionAfterTcpServerGoAwayScenarioTest.java +++ b/native/src/test/java/io/ballerina/stdlib/http/transport/http2/frameleveltests/Http2ChannelCloseBeforeConnectionEvictionAfterTcpServerGoAwayScenarioTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.com) All Rights Reserved. + * Copyright (c) 2024, WSO2 LLC. (http://www.wso2.com). * * WSO2 LLC. licenses this file to you under the Apache License, * Version 2.0 (the "License"); you may not use this file except