This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit e7d6cc49d6800c9bfd43b1789946e1062727b933 Author: Masaori Koshiba <[email protected]> AuthorDate: Thu Sep 25 09:09:46 2025 +0900 Fix connection leak on client abort (#12529) (cherry picked from commit 1754d38c88327d4b61107c93145ea36f88598167) --- src/proxy/http/HttpSM.cc | 19 +++-- .../gold_tests/h2/http2_concurrent_streams.test.py | 58 ++++++++++++++ tests/gold_tests/h2/http2_rst_stream.test.py | 2 +- .../h2/replay/http2_concurrent_streams.replay.yaml | 93 ++++++++++++++++++++++ 4 files changed, 166 insertions(+), 6 deletions(-) diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index d690333460..71868c0f61 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -868,15 +868,24 @@ HttpSM::state_watch_for_client_abort(int event, void *data) * client. */ case VC_EVENT_EOS: { - // We got an early EOS. To trigger background fill, do NOT kill HttpSM. + // We got an early EOS. if (!terminate_sm) { // Not done already NetVConnection *netvc = _ua.get_txn()->get_netvc(); - if (netvc) { - if (_ua.get_txn()->allow_half_open()) { + if (_ua.get_txn()->allow_half_open() || tunnel.has_consumer_besides_client()) { + if (netvc) { netvc->do_io_shutdown(IO_SHUTDOWN_READ); - } else { - netvc->do_io_shutdown(IO_SHUTDOWN_READWRITE); } + } else if (t_state.txn_conf->cache_http && + (server_entry != nullptr && server_entry->vc_read_handler == &HttpSM::state_read_server_response_header)) { + // if HttpSM is waiting response header from origin server, keep it for a while to run background fetch + _ua.get_txn()->do_io_shutdown(IO_SHUTDOWN_READWRITE); + } else { + _ua.get_txn()->do_io_close(); + vc_table.cleanup_entry(_ua.get_entry()); + _ua.set_entry(nullptr); + tunnel.kill_tunnel(); + terminate_sm = true; // Just die already, the requester is gone + set_ua_abort(HttpTransact::ABORTED, event); } if (_ua.get_entry()) { _ua.get_entry()->eos = true; diff --git a/tests/gold_tests/h2/http2_concurrent_streams.test.py b/tests/gold_tests/h2/http2_concurrent_streams.test.py new file mode 100644 index 0000000000..7510b8e777 --- /dev/null +++ b/tests/gold_tests/h2/http2_concurrent_streams.test.py @@ -0,0 +1,58 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF 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. + +import sys + +Test.Sumary = ''' +Verify Concurrent Streams Handling +''' + + +class Http2ConcurrentStreamsTest: + replayFile = "replay/http2_concurrent_streams.replay.yaml" + + def __init__(self): + self.__setupOriginServer() + self.__setupTS() + + def __setupOriginServer(self): + self._server = Test.MakeVerifierServerProcess("verifier-server", self.replayFile) + + def __setupTS(self): + self._ts = Test.MakeATSProcess(f"ts", enable_tls=True, enable_cache=True) + self._ts.addDefaultSSLFiles() + self._ts.Disk.records_config.update( + { + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'http2', + 'proxy.config.ssl.server.cert.path': f"{self._ts.Variables.SSLDir}", + 'proxy.config.ssl.server.private_key.path': f"{self._ts.Variables.SSLDir}", + 'proxy.config.http.insert_response_via_str': 2, + }) + self._ts.Disk.remap_config.AddLine(f"map / http://127.0.0.1:{self._server.Variables.http_port}";) + self._ts.Disk.ssl_multicert_config.AddLine('dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key') + + def run(self): + tr = Test.AddTestRun() + tr.AddVerifierClientProcess( + "verifier-client", self.replayFile, http_ports=[self._ts.Variables.port], https_ports=[self._ts.Variables.ssl_port]) + tr.Processes.Default.StartBefore(self._ts) + tr.Processes.Default.StartBefore(self._server) + tr.StillRunningAfter = self._ts + tr.StillRunningAfter = self._server + + +Http2ConcurrentStreamsTest().run() diff --git a/tests/gold_tests/h2/http2_rst_stream.test.py b/tests/gold_tests/h2/http2_rst_stream.test.py index 1b5c6bb6cd..caf2a71141 100644 --- a/tests/gold_tests/h2/http2_rst_stream.test.py +++ b/tests/gold_tests/h2/http2_rst_stream.test.py @@ -58,7 +58,7 @@ tr.Processes.Default.Streams.All += Testers.ContainsExpression( tr.Processes.Default.Streams.All += Testers.ContainsExpression( 'Submitted RST_STREAM frame for key 1 on stream 1.', 'Send RST_STREAM frame.') -server.Streams.All += Testers.ContainsExpression('RST_STREAM', 'Origin Server received RST_STREAM frame.') +server.Streams.All += Testers.ExcludesExpression('RST_STREAM', 'Server is not affected.') ts.Disk.traffic_out.Content += Testers.ContainsExpression('Received HEADERS frame', 'Received HEADERS frame.') diff --git a/tests/gold_tests/h2/replay/http2_concurrent_streams.replay.yaml b/tests/gold_tests/h2/replay/http2_concurrent_streams.replay.yaml new file mode 100644 index 0000000000..4747f7eb63 --- /dev/null +++ b/tests/gold_tests/h2/replay/http2_concurrent_streams.replay.yaml @@ -0,0 +1,93 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF 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. + +# +# This replay file assumes that caching is enabled and +# proxy.config.http.cache.ignore_client_cc_max_age is set to 0 so that we can +# test max-age in the client requests. +# + +meta: + version: "1.0" + +sessions: +- protocol: + - name: http + version: 2 + - name: tls + sni: test_sni + - name: tcp + - name: ip + transactions: + # Stream 1 + - client-request: + frames: + - HEADERS: + headers: + fields: + - [ :method, GET ] + - [ :scheme, https ] + - [ :authority, www.example.com ] + - [ :path, /path/1 ] + - [ Content-Type, image/jpeg ] + - [ uuid, 1 ] + - RST_STREAM: + delay: 1s + error-code: CANCEL + + server-response: + delay: 2s + headers: + fields: + - [ :status, 200 ] + - [ Content-Type, image/jpeg ] + - [ X-Response, first-response ] + - [ Cache-Control, "private" ] + content: + size: 1024 + + # Stream 3 + - client-request: + headers: + fields: + - [ :method, GET ] + - [ :scheme, https ] + - [ :authority, www.example.com ] + - [ :path, /path/3 ] + - [ Content-Type, image/jpeg ] + - [ uuid, 3] + + server-response: + delay: 1s + + headers: + fields: + - [ :status, 200 ] + - [ Content-Type, image/jpeg ] + - [ X-Response, second-response ] + - [ Cache-Control, "private" ] + content: + size: 1024 + + proxy-response: + headers: + fields: + - [ :status, 200 ] + - [ Content-Type, image/jpeg ] + - [ X-Response, second-response ] + - [ Cache-Control, "private" ] + content: + size: 1024
