diff --git a/src/iocore/net/P_SSLNetVConnection.h b/src/iocore/net/P_SSLNetVConnection.h index c0e157b8b28..dea3328f688 100644 --- a/src/iocore/net/P_SSLNetVConnection.h +++ b/src/iocore/net/P_SSLNetVConnection.h @@ -128,6 +128,7 @@ class SSLNetVConnection : public UnixNetVConnection, void net_read_io(NetHandler *nh) override; int64_t load_buffer_and_write(int64_t towrite, MIOBufferAccessor &buf, int64_t &total_written, int &needs) override; void do_io_close(int lerrno = -1) override; + void do_io_shutdown(ShutdownHowTo_t howto) override; //////////////////////////////////////////////////////////// // Instances of NetVConnection should be allocated // diff --git a/src/iocore/net/SSLNetVConnection.cc b/src/iocore/net/SSLNetVConnection.cc index d3e3c2aae1e..4d174d258c5 100644 --- a/src/iocore/net/SSLNetVConnection.cc +++ b/src/iocore/net/SSLNetVConnection.cc @@ -23,6 +23,7 @@ #include "BIO_fastopen.h" #include "P_UnixNet.h" +#include "P_UnixNetVConnection.h" #include "SSLStats.h" #include "P_Net.h" #include "P_SSLUtils.h" @@ -36,6 +37,7 @@ #include "iocore/net/ProxyProtocol.h" #include "iocore/net/SSLDiags.h" #include "iocore/net/SSLSNIConfig.h" +#include "iocore/net/SSLTypes.h" #include "iocore/net/TLSALPNSupport.h" #include "tscore/ink_config.h" #include "tscore/Layout.h" @@ -897,6 +899,51 @@ SSLNetVConnection::do_io_close(int lerrno) super::do_io_close(lerrno); } +void +SSLNetVConnection::do_io_shutdown(ShutdownHowTo_t howto) +{ + if (get_tunnel_type() == SNIRoutingType::BLIND) { + // we don't have TLS layer control of blind tunnel + UnixNetVConnection::do_io_shutdown(howto); + return; + } + + switch (howto) { + case IO_SHUTDOWN_READ: + // No need to call SSL API + // SSL_shutdown() sends the close_notify alert to the peer and it only closes the write direction. + // The read direction will be closed by the peer. + read.enabled = 0; + read.vio.buffer.clear(); + read.vio.nbytes = 0; + read.vio.cont = nullptr; + f.shutdown |= NetEvent::SHUTDOWN_READ; + break; + case IO_SHUTDOWN_WRITE: + SSL_shutdown(ssl); + write.enabled = 0; + write.vio.buffer.clear(); + write.vio.nbytes = 0; + write.vio.cont = nullptr; + f.shutdown |= NetEvent::SHUTDOWN_WRITE; + break; + case IO_SHUTDOWN_READWRITE: + SSL_shutdown(ssl); + read.enabled = 0; + write.enabled = 0; + read.vio.buffer.clear(); + read.vio.nbytes = 0; + write.vio.buffer.clear(); + write.vio.nbytes = 0; + read.vio.cont = nullptr; + write.vio.cont = nullptr; + f.shutdown = NetEvent::SHUTDOWN_READ | NetEvent::SHUTDOWN_WRITE; + break; + default: + ink_assert(!"not reached"); + } +} + void SSLNetVConnection::clear() { diff --git a/src/iocore/net/UnixNetVConnection.cc b/src/iocore/net/UnixNetVConnection.cc index 79e427011fb..7285e618f9b 100644 --- a/src/iocore/net/UnixNetVConnection.cc +++ b/src/iocore/net/UnixNetVConnection.cc @@ -625,6 +625,11 @@ UnixNetVConnection::net_write_io(NetHandler *nh) NetState *s = &this->write; Continuation *c = this->write.vio.cont; + if (c == nullptr) { + // If Continuation to callback is nullptr, we can do nothing + return; + } + MUTEX_TRY_LOCK(lock, s->vio.mutex, thread); if (!lock.is_locked() || lock.get_mutex() != s->vio.mutex.get()) { diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index 7c15aed2329..35baef13836 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -869,20 +869,15 @@ HttpSM::state_watch_for_client_abort(int event, void *data) * client. */ case VC_EVENT_EOS: { - // We got an early EOS. If the tunnal has cache writer, don't kill it for background fill. + // We got an early EOS. To trigger background fill, do NOT kill HttpSM. if (!terminate_sm) { // Not done already NetVConnection *netvc = _ua.get_txn()->get_netvc(); - if (_ua.get_txn()->allow_half_open() || tunnel.has_consumer_besides_client()) { - if (netvc) { + if (netvc) { + if (_ua.get_txn()->allow_half_open()) { netvc->do_io_shutdown(IO_SHUTDOWN_READ); + } else { + netvc->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/cache/background_fill.test.py b/tests/gold_tests/cache/background_fill.test.py index 4c4fe2467d6..abf09fbb802 100644 --- a/tests/gold_tests/cache/background_fill.test.py +++ b/tests/gold_tests/cache/background_fill.test.py @@ -72,6 +72,8 @@ def __setupTS(self, ts_names=['default']): "proxy.config.diags.debug.tags": "http", }) + self.ts[name].Disk.plugin_config.AddLine('xdebug.so --enable=x-cache') + if name == 'for_httpbin' or name == 'default': self.ts[name].Disk.remap_config.AddLines([ f"map / http://127.0.0.1:{self.httpbin.Variables.Port}", @@ -110,14 +112,13 @@ def __testCase0(self): tr.MakeCurlCommandMulti( f""" {{curl}} -X PURGE --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4; -timeout 2 {{curl}} --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4; -sleep 4; -{{curl}} --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4 +timeout 1 {{curl}} --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4; +sleep 5; +{{curl}} --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4 -H "x-debug: x-cache" """, ts=self.ts['for_httpbin']) tr.Processes.Default.ReturnCode = 0 - tr.Processes.Default.Streams.stderr = Testers.Any( - "gold/background_fill_0_stderr_H.gold", "gold/background_fill_0_stderr_W.gold") + tr.Processes.Default.Streams.stderr = "gold/background_fill_0_stderr_H.gold" self.__checkProcessAfter(tr) def __testCase1(self): @@ -129,14 +130,13 @@ def __testCase1(self): tr.MakeCurlCommandMulti( f""" {{curl}} -X PURGE --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; -timeout 3 {{curl}} --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; +timeout 1 {{curl}} --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; sleep 5; -{{curl}} --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4 +{{curl}} --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4 -H "x-debug: x-cache" """, ts=self.ts['for_httpbin']) tr.Processes.Default.ReturnCode = 0 - tr.Processes.Default.Streams.stderr = Testers.Any( - "gold/background_fill_1_stderr_H.gold", "gold/background_fill_1_stderr_W.gold") + tr.Processes.Default.Streams.stderr = "gold/background_fill_1_stderr_H.gold" self.__checkProcessAfter(tr) def __testCase2(self): @@ -148,14 +148,13 @@ def __testCase2(self): tr.MakeCurlCommandMulti( f""" {{curl}} -X PURGE --http2 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; -timeout 3 {{curl}} --http2 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; +timeout 1 {{curl}} --http2 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4; sleep 5; -{{curl}} --http2 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4 +{{curl}} --http2 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4 -H "x-debug: x-cache" """, ts=self.ts['for_httpbin']) tr.Processes.Default.ReturnCode = 0 - tr.Processes.Default.Streams.stderr = Testers.Any( - "gold/background_fill_2_stderr_H.gold", "gold/background_fill_2_stderr_W.gold") + tr.Processes.Default.Streams.stderr = "gold/background_fill_2_stderr_H.gold" self.__checkProcessAfter(tr) def __testCase3(self): diff --git a/tests/gold_tests/cache/gold/background_fill_0_stderr_H.gold b/tests/gold_tests/cache/gold/background_fill_0_stderr_H.gold index 6c9b90e886d..3a71e8ea161 100644 --- a/tests/gold_tests/cache/gold/background_fill_0_stderr_H.gold +++ b/tests/gold_tests/cache/gold/background_fill_0_stderr_H.gold @@ -7,11 +7,9 @@ `` > GET /drip?duration=4 HTTP/1.1 `` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cMsSfW]) -`` > GET /drip?duration=4 HTTP/1.1 `` -< HTTP/1.1 `` +< HTTP/1.1 200 OK `` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cHs f ]) +< X-Cache: hit-fresh `` diff --git a/tests/gold_tests/cache/gold/background_fill_0_stderr_W.gold b/tests/gold_tests/cache/gold/background_fill_0_stderr_W.gold deleted file mode 100644 index 383c9c73647..00000000000 --- a/tests/gold_tests/cache/gold/background_fill_0_stderr_W.gold +++ /dev/null @@ -1,15 +0,0 @@ -`` -> PURGE /drip?duration=4 HTTP/1.1 -`` -< HTTP/1.1 `` -`` -< Via: http/1.1 traffic_server (ApacheTrafficServer/``) -`` -> GET /drip?duration=4 HTTP/1.1 -`` -> GET /drip?duration=4 HTTP/1.1 -`` -< HTTP/1.1 `` -`` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cWs f ]) -`` diff --git a/tests/gold_tests/cache/gold/background_fill_1_stderr_H.gold b/tests/gold_tests/cache/gold/background_fill_1_stderr_H.gold index 6c9b90e886d..3a71e8ea161 100644 --- a/tests/gold_tests/cache/gold/background_fill_1_stderr_H.gold +++ b/tests/gold_tests/cache/gold/background_fill_1_stderr_H.gold @@ -7,11 +7,9 @@ `` > GET /drip?duration=4 HTTP/1.1 `` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cMsSfW]) -`` > GET /drip?duration=4 HTTP/1.1 `` -< HTTP/1.1 `` +< HTTP/1.1 200 OK `` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cHs f ]) +< X-Cache: hit-fresh `` diff --git a/tests/gold_tests/cache/gold/background_fill_1_stderr_W.gold b/tests/gold_tests/cache/gold/background_fill_1_stderr_W.gold deleted file mode 100644 index 383c9c73647..00000000000 --- a/tests/gold_tests/cache/gold/background_fill_1_stderr_W.gold +++ /dev/null @@ -1,15 +0,0 @@ -`` -> PURGE /drip?duration=4 HTTP/1.1 -`` -< HTTP/1.1 `` -`` -< Via: http/1.1 traffic_server (ApacheTrafficServer/``) -`` -> GET /drip?duration=4 HTTP/1.1 -`` -> GET /drip?duration=4 HTTP/1.1 -`` -< HTTP/1.1 `` -`` -< Via: http/1.1 traffic_server (ApacheTrafficServer/`` [cWs f ]) -`` diff --git a/tests/gold_tests/cache/gold/background_fill_2_stderr_H.gold b/tests/gold_tests/cache/gold/background_fill_2_stderr_H.gold index ed4459477bb..afa508e71c6 100644 --- a/tests/gold_tests/cache/gold/background_fill_2_stderr_H.gold +++ b/tests/gold_tests/cache/gold/background_fill_2_stderr_H.gold @@ -7,11 +7,9 @@ `` > GET /drip?duration=4 HTTP/2 `` -< via: http/1.1 traffic_server (ApacheTrafficServer/`` [cMsSfW]) -`` > GET /drip?duration=4 HTTP/2 `` -< HTTP/2 `` +< HTTP/2 200`` `` -< via: http/1.1 traffic_server (ApacheTrafficServer/`` [cHs f ]) +< x-cache: hit-fresh `` diff --git a/tests/gold_tests/cache/gold/background_fill_2_stderr_W.gold b/tests/gold_tests/cache/gold/background_fill_2_stderr_W.gold deleted file mode 100644 index bff1dbb288d..00000000000 --- a/tests/gold_tests/cache/gold/background_fill_2_stderr_W.gold +++ /dev/null @@ -1,15 +0,0 @@ -`` -> PURGE /drip?duration=4 HTTP/2 -`` -< HTTP/2 `` -`` -< via: http/1.1 traffic_server (ApacheTrafficServer/``) -`` -> GET /drip?duration=4 HTTP/2 -`` -> GET /drip?duration=4 HTTP/2 -`` -< HTTP/2 `` -`` -< via: http/1.1 traffic_server (ApacheTrafficServer/`` [cWs f ]) -`` diff --git a/tests/gold_tests/cache/gold/background_fill_3_stdout.gold b/tests/gold_tests/cache/gold/background_fill_3_stdout.gold index b0c5ed4be6d..a492c2f88fe 100644 --- a/tests/gold_tests/cache/gold/background_fill_3_stdout.gold +++ b/tests/gold_tests/cache/gold/background_fill_3_stdout.gold @@ -4,12 +4,12 @@ content-type: text/html content-length: 11 `` -via: https/2 traffic_server (ApacheTrafficServer/`` [cMsSfW]) +x-cache: miss `` [DEBUG]: Received an HTTP/2 response for key 2 with stream id 1: :status: 200 content-type: text/html content-length: 11 `` -via: http/1.1 traffic_server (ApacheTrafficServer/`` [cHs f ]) +x-cache: hit-fresh `` diff --git a/tests/gold_tests/cache/replay/bg_fill.yaml b/tests/gold_tests/cache/replay/bg_fill.yaml index 6a76f8a2be3..f01a4f47a4e 100644 --- a/tests/gold_tests/cache/replay/bg_fill.yaml +++ b/tests/gold_tests/cache/replay/bg_fill.yaml @@ -44,6 +44,7 @@ sessions: - [:path, /a/path] - [Content-Type, text/html] - [uuid, 1] + - [x-debug, x-cache] - RST_STREAM: delay: 1s error-code: INTERNAL_ERROR @@ -84,6 +85,7 @@ sessions: - [:path, /a/path] - [Content-Type, text/html] - [uuid, 2] + - [x-debug, x-cache] server-response: frames: diff --git a/tests/gold_tests/h2/http2_rst_stream.test.py b/tests/gold_tests/h2/http2_rst_stream.test.py index caf2a711412..1b5c6bb6cd3 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( 'Submitted RST_STREAM frame for key 1 on stream 1.', 'Send RST_STREAM frame.') -server.Streams.All += Testers.ExcludesExpression('RST_STREAM', 'Server is not affected.') +server.Streams.All += Testers.ContainsExpression('RST_STREAM', 'Origin Server received RST_STREAM frame.') ts.Disk.traffic_out.Content += Testers.ContainsExpression('Received HEADERS frame', 'Received HEADERS frame.')