You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2023/05/01 09:41:42 UTC

[arrow] branch main updated: GH-35379: [C++][FlightRPC] Add teardown needed checks to avoid crash on error (#35380)

This is an automated email from the ASF dual-hosted git repository.

kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git


The following commit(s) were added to refs/heads/main by this push:
     new 41adf00652 GH-35379: [C++][FlightRPC] Add teardown needed checks to avoid crash on error (#35380)
41adf00652 is described below

commit 41adf00652bf6788581f9adc4c007af5c1de3d56
Author: Sutou Kouhei <ko...@clear-code.com>
AuthorDate: Mon May 1 18:41:35 2023 +0900

    GH-35379: [C++][FlightRPC] Add teardown needed checks to avoid crash on error (#35380)
    
    ### Rationale for this change
    
    If `ARROW_TEST_DATA` isn't set, `arrow-flight-test` is crashed:
    
    ```console
    $ LD_LIBRARY_PATH=$PWD/debug debug/arrow-flight-test
    ...
    [----------] 4 tests from TestTls
    [ RUN      ] TestTls.DoAction
    E0501 16:24:25.455014704  800882 ssl_security_connector.cc:270]        Handshaker factory creation failed with TSI_INVALID_ARGUMENT.
    E0501 16:24:25.455054228  800882 chttp2_server.cc:1045]                UNKNOWN:Unable to create secure server with credentials of type Ssl {file:"./src/core/ext/transport/chttp2/server/chttp2_server.cc", file_line:1032, created_time:"2023-05-01T16:24:25.455046974+09:00"}
    /home/kou/work/cpp/arrow.kou/cpp/src/arrow/flight/flight_test.cc:396: Failure
    Failed
    'ExampleTlsCertificates(&options.tls_certificates)' failed with IOError: Test resources not found, set ARROW_TEST_DATA to <repo root>/testing/data
    ../cpp/src/arrow/flight/test_util.cc:784  GetTestResourceRoot(&root)
    ```
    
    Because `client_` isn't created and `server_->Init()` isn't called when this error is occurred and `TearDown()` is executed.
    
    ### What changes are included in this PR?
    
    Executes each teardown process only when they are needed.
    
    ### Are these changes tested?
    
    Yes.
    
    ### Are there any user-facing changes?
    
    No.
    * Closes: #35379
    
    Authored-by: Sutou Kouhei <ko...@clear-code.com>
    Signed-off-by: Sutou Kouhei <ko...@clear-code.com>
---
 cpp/src/arrow/flight/flight_test.cc | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/cpp/src/arrow/flight/flight_test.cc b/cpp/src/arrow/flight/flight_test.cc
index a2b69494b8..09e9d8c156 100644
--- a/cpp/src/arrow/flight/flight_test.cc
+++ b/cpp/src/arrow/flight/flight_test.cc
@@ -419,14 +419,19 @@ class TestTls : public ::testing::Test {
     ASSERT_RAISES(UnknownError, server_->Init(options));
     ASSERT_OK(ExampleTlsCertificates(&options.tls_certificates));
     ASSERT_OK(server_->Init(options));
+    server_is_initialized_ = true;
 
     ASSERT_OK_AND_ASSIGN(location_, Location::ForGrpcTls("localhost", server_->port()));
     ASSERT_OK(ConnectClient());
   }
 
   void TearDown() {
-    ASSERT_OK(client_->Close());
-    ASSERT_OK(server_->Shutdown());
+    if (client_) {
+      ASSERT_OK(client_->Close());
+    }
+    if (server_is_initialized_) {
+      ASSERT_OK(server_->Shutdown());
+    }
     grpc_shutdown();
   }
 
@@ -442,6 +447,7 @@ class TestTls : public ::testing::Test {
   Location location_;
   std::unique_ptr<FlightClient> client_;
   std::unique_ptr<FlightServerBase> server_;
+  bool server_is_initialized_;
 };
 
 // A server middleware that rejects all calls.