Fix background fill on TLS connections

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        //

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()
 {

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()) {

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;

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):

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
 ``

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
 ``

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
 ``

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
 ``

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:

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.')