Skip to content

Commit 1754d38

Browse files
masaori335masaori335
authored
Fix connection leak on client abort (#12529)
1 parent 67b029b commit 1754d38

File tree

4 files changed

+166
-6
lines changed

4 files changed

+166
-6
lines changed

‎src/proxy/http/HttpSM.cc‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -869,15 +869,24 @@ HttpSM::state_watch_for_client_abort(int event, void *data)
869869
* client.
870870
*/
871871
case VC_EVENT_EOS: {
872-
// We got an early EOS. To trigger background fill, do NOT kill HttpSM.
872+
// We got an early EOS.
873873
if (!terminate_sm) { // Not done already
874874
NetVConnection *netvc = _ua.get_txn()->get_netvc();
875-
if (netvc) {
876-
if (_ua.get_txn()->allow_half_open()) {
875+
if (_ua.get_txn()->allow_half_open() || tunnel.has_consumer_besides_client()) {
876+
if (netvc) {
877877
netvc->do_io_shutdown(IO_SHUTDOWN_READ);
878-
} else {
879-
netvc->do_io_shutdown(IO_SHUTDOWN_READWRITE);
880878
}
879+
} else if (t_state.txn_conf->cache_http &&
880+
(server_entry != nullptr && server_entry->vc_read_handler == &HttpSM::state_read_server_response_header)) {
881+
// if HttpSM is waiting response header from origin server, keep it for a while to run background fetch
882+
_ua.get_txn()->do_io_shutdown(IO_SHUTDOWN_READWRITE);
883+
} else {
884+
_ua.get_txn()->do_io_close();
885+
vc_table.cleanup_entry(_ua.get_entry());
886+
_ua.set_entry(nullptr);
887+
tunnel.kill_tunnel();
888+
terminate_sm = true; // Just die already, the requester is gone
889+
set_ua_abort(HttpTransact::ABORTED, event);
881890
}
882891
if (_ua.get_entry()) {
883892
_ua.get_entry()->eos = true;

‎tests/gold_tests/h2/http2_concurrent_streams.test.py‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
import sys
18+
19+
Test.Sumary = '''
20+
Verify Concurrent Streams Handling
21+
'''
22+
23+
24+
class Http2ConcurrentStreamsTest:
25+
replayFile = "replay/http2_concurrent_streams.replay.yaml"
26+
27+
def __init__(self):
28+
self.__setupOriginServer()
29+
self.__setupTS()
30+
31+
def __setupOriginServer(self):
32+
self._server = Test.MakeVerifierServerProcess("verifier-server", self.replayFile)
33+
34+
def __setupTS(self):
35+
self._ts = Test.MakeATSProcess(f"ts", enable_tls=True, enable_cache=True)
36+
self._ts.addDefaultSSLFiles()
37+
self._ts.Disk.records_config.update(
38+
{
39+
'proxy.config.diags.debug.enabled': 1,
40+
'proxy.config.diags.debug.tags': 'http2',
41+
'proxy.config.ssl.server.cert.path': f"{self._ts.Variables.SSLDir}",
42+
'proxy.config.ssl.server.private_key.path': f"{self._ts.Variables.SSLDir}",
43+
'proxy.config.http.insert_response_via_str': 2,
44+
})
45+
self._ts.Disk.remap_config.AddLine(f"map / http://127.0.0.1:{self._server.Variables.http_port}")
46+
self._ts.Disk.ssl_multicert_config.AddLine('dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key')
47+
48+
def run(self):
49+
tr = Test.AddTestRun()
50+
tr.AddVerifierClientProcess(
51+
"verifier-client", self.replayFile, http_ports=[self._ts.Variables.port], https_ports=[self._ts.Variables.ssl_port])
52+
tr.Processes.Default.StartBefore(self._ts)
53+
tr.Processes.Default.StartBefore(self._server)
54+
tr.StillRunningAfter = self._ts
55+
tr.StillRunningAfter = self._server
56+
57+
58+
Http2ConcurrentStreamsTest().run()

‎tests/gold_tests/h2/http2_rst_stream.test.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
tr.Processes.Default.Streams.All += Testers.ContainsExpression(
5959
'Submitted RST_STREAM frame for key 1 on stream 1.', 'Send RST_STREAM frame.')
6060

61-
server.Streams.All += Testers.ContainsExpression('RST_STREAM', 'Origin Server received RST_STREAM frame.')
61+
server.Streams.All += Testers.ExcludesExpression('RST_STREAM', 'Server is not affected.')
6262

6363
ts.Disk.traffic_out.Content += Testers.ContainsExpression('Received HEADERS frame', 'Received HEADERS frame.')
6464

‎tests/gold_tests/h2/replay/http2_concurrent_streams.replay.yaml‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
#
18+
# This replay file assumes that caching is enabled and
19+
# proxy.config.http.cache.ignore_client_cc_max_age is set to 0 so that we can
20+
# test max-age in the client requests.
21+
#
22+
23+
meta:
24+
version: "1.0"
25+
26+
sessions:
27+
- protocol:
28+
- name: http
29+
version: 2
30+
- name: tls
31+
sni: test_sni
32+
- name: tcp
33+
- name: ip
34+
transactions:
35+
# Stream 1
36+
- client-request:
37+
frames:
38+
- HEADERS:
39+
headers:
40+
fields:
41+
- [ :method, GET ]
42+
- [ :scheme, https ]
43+
- [ :authority, www.example.com ]
44+
- [ :path, /path/1 ]
45+
- [ Content-Type, image/jpeg ]
46+
- [ uuid, 1 ]
47+
- RST_STREAM:
48+
delay: 1s
49+
error-code: CANCEL
50+
51+
server-response:
52+
delay: 2s
53+
headers:
54+
fields:
55+
- [ :status, 200 ]
56+
- [ Content-Type, image/jpeg ]
57+
- [ X-Response, first-response ]
58+
- [ Cache-Control, "private" ]
59+
content:
60+
size: 1024
61+
62+
# Stream 3
63+
- client-request:
64+
headers:
65+
fields:
66+
- [ :method, GET ]
67+
- [ :scheme, https ]
68+
- [ :authority, www.example.com ]
69+
- [ :path, /path/3 ]
70+
- [ Content-Type, image/jpeg ]
71+
- [ uuid, 3]
72+
73+
server-response:
74+
delay: 1s
75+
76+
headers:
77+
fields:
78+
- [ :status, 200 ]
79+
- [ Content-Type, image/jpeg ]
80+
- [ X-Response, second-response ]
81+
- [ Cache-Control, "private" ]
82+
content:
83+
size: 1024
84+
85+
proxy-response:
86+
headers:
87+
fields:
88+
- [ :status, 200 ]
89+
- [ Content-Type, image/jpeg ]
90+
- [ X-Response, second-response ]
91+
- [ Cache-Control, "private" ]
92+
content:
93+
size: 1024

0 commit comments

Comments
 (0)