fix core: handle http2 GOAWAY

commit_hash:1bae7472ddacc5036cc9b19235297d9f5e35ffce

Author: ArkadyRudenko

core/functional_tests/http2server/tests/test_h2spec.py

@@ -1,8 +1,8 @@
-# h2spec compliance tests https://github.com/summerwind/h2spec/tree/master/http2
+# h2spec compliance tests https://github.com/summerwind/h2spec
 #
-# Names mirror h2spec spec IDs: http2/<section>/<case>.
-# Section maps to a source file, e.g. http2/6_7_ping.go -> RFC 7540 §6.7 (PING).
-# Case is the 1-based index of AddTestCase in that file; run via `h2spec http2/6.7/2`.
+# Names mirror h2spec spec IDs: <spec>/<section>/<case>.
+# Section maps to a source file, e.g. http2/6_7_ping.go or generic/3_8_goaway.go.
+# Case is the 1-based index of AddTestCase in that file.
 import asyncio
 
 import pytest
@@ -21,3 +21,21 @@ async def test_h2spec_6_7_2_ping_ack_ignored(create_connection, service_client):
 
         with pytest.raises(asyncio.TimeoutError):
             await sock.recv(utils.RECEIVE_SIZE, timeout=0.5)
+
+
+async def test_h2spec_generic_3_8_1_client_goaway(create_connection, monitor_client, service_client):
+    # generic/3_8_goaway.go, case 1 ("Sends a GOAWAY frame"):
+    # after client GOAWAY the server must close the connection or answer a follow-up PING.
+    await service_client.update_server_state()
+
+    async with monitor_client.metrics_diff(prefix='server.requests.http2') as differ:
+        async with create_connection() as (sock, conn):
+            conn.close_connection(error_code=0)
+            ping_data = b'h2spec\x00\x00'
+            ping = utils.create_frame(utils.PING_FRAME, utils.EMPTY_FLAGS, 0, ping_data)
+            await sock.sendall(conn.data_to_send() + ping)
+
+            receive = await sock.recv(utils.RECEIVE_SIZE, timeout=2.0)
+            assert receive == b''
+
+    assert differ.value_at('goaway') == 1

core/functional_tests/http2server/tests/test_low_level.py

@@ -1,5 +1,4 @@
 import asyncio
-import contextlib
 import struct
 
 import h2.connection
@@ -425,25 +424,6 @@ async def test_single_reset_keeps_connection_usable(
     assert differ.value_at('reset-streams') == 1
 
 
-async def test_client_goaway_metric(create_connection, monitor_client, service_client):
-    await service_client.update_server_state()
-
-    async with monitor_client.metrics_diff(prefix='server.requests.http2') as differ:
-        async with create_connection() as (sock, conn):
-            conn.close_connection(error_code=0)
-            await sock.sendall(conn.data_to_send())
-            # Keep the socket open until the server reads GOAWAY (and optionally
-            # responds). Closing immediately races with metrics collection.
-            with contextlib.suppress(asyncio.TimeoutError):
-                while True:
-                    receive = await sock.recv(utils.RECEIVE_SIZE, timeout=1.0)
-                    if not receive:
-                        break
-                    conn.receive_data(receive)
-
-    assert differ.value_at('goaway') == 1
-
-
 async def test_stream_already_closed(create_connection, service_client):
     async with create_connection() as (sock, conn):
 

core/src/server/http/http2_session.cpp

@@ -353,7 +353,7 @@ void Http2Session::FinalizeRequest(Stream& stream) {
     }
 }
 
-bool Http2Session::ConnectionIsOk() {
+bool Http2Session::ConnectionIsOk() const {
     return nghttp2_session_want_read(session_.get()) != 0 || nghttp2_session_want_write(session_.get()) != 0;
 }
 

core/src/server/http/http2_session.hpp

@@ -64,6 +64,8 @@ class Http2Session final : public request::RequestParser {
     void WriteWhileWant();
     void HandleStreamingEvents();
 
+    bool ConnectionIsOk() const;
+
 private:
     friend class Http2ResponseWriter;
 
@@ -113,7 +115,6 @@ class Http2Session final : public request::RequestParser {
     void SubmitRstStream(Stream::Id stream_id);
 
     void FinalizeRequest(Stream& stream);
-    bool ConnectionIsOk();
 
     const net::Http2SessionConfig& config_;
 

core/src/server/net/http2_connection.cpp

@@ -96,6 +96,10 @@ void Http2Connection::ListenForRequests() {
     while (!engine::current_task::ShouldCancel()) {
         StartAllRequestTasks(wait_any);
 
+        if (ShouldCloseConnection()) {
+            return;
+        }
+
         const auto ready_id = wait_any.Wait();
         if (!ready_id) {
             UASSERT(ready_id == utils::unexpected(engine::WaitAnyError::kCancelled));
@@ -178,6 +182,12 @@ void Http2Connection::SendResponse(http::HttpRequest& request) noexcept {
     request.WriteAccessLogs(request_handler_.LoggerAccess(), request_handler_.LoggerAccessTskv(), GetPeerName());
 }
 
+bool Http2Connection::ShouldCloseConnection() const noexcept {
+    UASSERT(parser_);
+    // Check handler_tasks_ to do graceful shutdown.
+    return !parser_->ConnectionIsOk() && handler_tasks_.empty();
+}
+
 std::unique_ptr<http::Http2Session> Http2Connection::MakeParser() {
     const auto on_req_cb = [this](HttpRequestPtr&& request_ptr) {
         pending_requests_.push_back(std::move(request_ptr));

core/src/server/net/http2_connection.hpp

@@ -68,6 +68,8 @@ class Http2Connection final : public ConnectionBase {
     std::unique_ptr<http::Http2Session> MakeParser();
     void EnsureHttp2();
 
+    bool ShouldCloseConnection() const noexcept;
+
     const ConnectionConfig& config_;
     const request::HttpRequestConfig& handler_defaults_config_;
     const http::RequestHandlerBase& request_handler_;