From 69d2477db044b927545a628d67e7bff20c0c1310 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Percy=20Camilo=20Trive=C3=B1o=20Aucahuasi?= Date: Tue, 25 Oct 2022 17:28:21 -0500 Subject: [PATCH 1/5] invoke google::protobuf::ShutdownProtobufLibrary to avoid valgrind mem leaks format invoke google::protobuf::ShutdownProtobufLibrary for all substrait tests --- cpp/src/arrow/engine/CMakeLists.txt | 1 + .../engine/substrait/protobuf_test_util.cc | 30 ++++++++++++++ .../engine/substrait/protobuf_test_util.h | 41 +++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 cpp/src/arrow/engine/substrait/protobuf_test_util.cc create mode 100644 cpp/src/arrow/engine/substrait/protobuf_test_util.h diff --git a/cpp/src/arrow/engine/CMakeLists.txt b/cpp/src/arrow/engine/CMakeLists.txt index a8d5be90af8..b79a25a7130 100644 --- a/cpp/src/arrow/engine/CMakeLists.txt +++ b/cpp/src/arrow/engine/CMakeLists.txt @@ -72,6 +72,7 @@ add_arrow_test(substrait_test substrait/ext_test.cc substrait/function_test.cc substrait/serde_test.cc + substrait/protobuf_test_util.cc EXTRA_LINK_LIBS ${ARROW_SUBSTRAIT_TEST_LINK_LIBS} PREFIX diff --git a/cpp/src/arrow/engine/substrait/protobuf_test_util.cc b/cpp/src/arrow/engine/substrait/protobuf_test_util.cc new file mode 100644 index 00000000000..4072f99bb46 --- /dev/null +++ b/cpp/src/arrow/engine/substrait/protobuf_test_util.cc @@ -0,0 +1,30 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "arrow/engine/substrait/protobuf_test_util.h" + +namespace arrow { +namespace engine { + +void ProtobufEnvironment::SetUp() { GOOGLE_PROTOBUF_VERIFY_VERSION; } +void ProtobufEnvironment::TearDown() { google::protobuf::ShutdownProtobufLibrary(); } + +::testing::Environment* protobuf_env = + ::testing::AddGlobalTestEnvironment(new ProtobufEnvironment); + +} // namespace engine +} // namespace arrow diff --git a/cpp/src/arrow/engine/substrait/protobuf_test_util.h b/cpp/src/arrow/engine/substrait/protobuf_test_util.h new file mode 100644 index 00000000000..8cbcbb211d2 --- /dev/null +++ b/cpp/src/arrow/engine/substrait/protobuf_test_util.h @@ -0,0 +1,41 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include +#include +#include + +#include + +#include + +namespace arrow { +namespace engine { + +// A global test "environment", to ensure that the Protobuf API is finalized after +// running unit tests. + +class ProtobufEnvironment : public ::testing::Environment { + public: + void SetUp() override; + void TearDown() override; +}; + +} // namespace engine +} // namespace arrow From 31b0dc88eac0e9f50f11162b83722da2a7061d46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Percy=20Camilo=20Trive=C3=B1o=20Aucahuasi?= Date: Wed, 26 Oct 2022 16:56:39 -0500 Subject: [PATCH 2/5] fix headers --- cpp/src/arrow/engine/substrait/protobuf_test_util.cc | 2 ++ cpp/src/arrow/engine/substrait/protobuf_test_util.h | 6 ------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/engine/substrait/protobuf_test_util.cc b/cpp/src/arrow/engine/substrait/protobuf_test_util.cc index 4072f99bb46..f60433a2d6b 100644 --- a/cpp/src/arrow/engine/substrait/protobuf_test_util.cc +++ b/cpp/src/arrow/engine/substrait/protobuf_test_util.cc @@ -17,6 +17,8 @@ #include "arrow/engine/substrait/protobuf_test_util.h" +#include + namespace arrow { namespace engine { diff --git a/cpp/src/arrow/engine/substrait/protobuf_test_util.h b/cpp/src/arrow/engine/substrait/protobuf_test_util.h index 8cbcbb211d2..48954417dd9 100644 --- a/cpp/src/arrow/engine/substrait/protobuf_test_util.h +++ b/cpp/src/arrow/engine/substrait/protobuf_test_util.h @@ -17,14 +17,8 @@ #pragma once -#include -#include -#include - #include -#include - namespace arrow { namespace engine { From a23c4557335e1fc5d062ee6ccdabe94daf390cab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Percy=20Camilo=20Trive=C3=B1o=20Aucahuasi?= Date: Wed, 26 Oct 2022 17:10:13 -0500 Subject: [PATCH 3/5] format --- cpp/src/arrow/engine/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/engine/CMakeLists.txt b/cpp/src/arrow/engine/CMakeLists.txt index b79a25a7130..09dff51f5fe 100644 --- a/cpp/src/arrow/engine/CMakeLists.txt +++ b/cpp/src/arrow/engine/CMakeLists.txt @@ -72,7 +72,7 @@ add_arrow_test(substrait_test substrait/ext_test.cc substrait/function_test.cc substrait/serde_test.cc - substrait/protobuf_test_util.cc + substrait/protobuf_test_util.cc EXTRA_LINK_LIBS ${ARROW_SUBSTRAIT_TEST_LINK_LIBS} PREFIX From 2ef66913dc815ea812c25810f2e5259891b4abb9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Percy=20Camilo=20Trive=C3=B1o=20Aucahuasi?= Date: Thu, 27 Oct 2022 22:38:16 -0500 Subject: [PATCH 4/5] simplify code --- .../engine/substrait/protobuf_test_util.cc | 11 +++--- .../engine/substrait/protobuf_test_util.h | 35 ------------------- 2 files changed, 7 insertions(+), 39 deletions(-) delete mode 100644 cpp/src/arrow/engine/substrait/protobuf_test_util.h diff --git a/cpp/src/arrow/engine/substrait/protobuf_test_util.cc b/cpp/src/arrow/engine/substrait/protobuf_test_util.cc index f60433a2d6b..ac34568f6e4 100644 --- a/cpp/src/arrow/engine/substrait/protobuf_test_util.cc +++ b/cpp/src/arrow/engine/substrait/protobuf_test_util.cc @@ -15,15 +15,18 @@ // specific language governing permissions and limitations // under the License. -#include "arrow/engine/substrait/protobuf_test_util.h" +#include -#include +#include namespace arrow { namespace engine { -void ProtobufEnvironment::SetUp() { GOOGLE_PROTOBUF_VERIFY_VERSION; } -void ProtobufEnvironment::TearDown() { google::protobuf::ShutdownProtobufLibrary(); } +class ProtobufEnvironment : public ::testing::Environment { + public: + void SetUp() override { GOOGLE_PROTOBUF_VERIFY_VERSION; } + void TearDown() override { google::protobuf::ShutdownProtobufLibrary(); } +}; ::testing::Environment* protobuf_env = ::testing::AddGlobalTestEnvironment(new ProtobufEnvironment); diff --git a/cpp/src/arrow/engine/substrait/protobuf_test_util.h b/cpp/src/arrow/engine/substrait/protobuf_test_util.h deleted file mode 100644 index 48954417dd9..00000000000 --- a/cpp/src/arrow/engine/substrait/protobuf_test_util.h +++ /dev/null @@ -1,35 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -#pragma once - -#include - -namespace arrow { -namespace engine { - -// A global test "environment", to ensure that the Protobuf API is finalized after -// running unit tests. - -class ProtobufEnvironment : public ::testing::Environment { - public: - void SetUp() override; - void TearDown() override; -}; - -} // namespace engine -} // namespace arrow From 9061fffc81c16709981538c07d50d6335e91dd9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Percy=20Camilo=20Trive=C3=B1o=20Aucahuasi?= Date: Fri, 28 Oct 2022 13:11:49 -0500 Subject: [PATCH 5/5] adding docs --- cpp/src/arrow/engine/substrait/protobuf_test_util.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cpp/src/arrow/engine/substrait/protobuf_test_util.cc b/cpp/src/arrow/engine/substrait/protobuf_test_util.cc index ac34568f6e4..d99622912ca 100644 --- a/cpp/src/arrow/engine/substrait/protobuf_test_util.cc +++ b/cpp/src/arrow/engine/substrait/protobuf_test_util.cc @@ -22,6 +22,13 @@ namespace arrow { namespace engine { +// A global test "environment", to ensure that the Protobuf API is finalized after +// running unit tests by invoking google::protobuf::ShutdownProtobufLibrary. +// This will prevent leaks in valgrind tests because it will delete the global objects +// that were allocated by the Protocol Buffer library (see +// "protobuf::ShutdownProtobufLibrary" in +// https://developers.google.com/protocol-buffers/docs/reference/cpp/google.protobuf.message_lite#ShutdownProtobufLibrary.details) + class ProtobufEnvironment : public ::testing::Environment { public: void SetUp() override { GOOGLE_PROTOBUF_VERIFY_VERSION; }