Skip to content

Commit 619b034

Browse files
authored
ARROW-17975: [C++] Create at-fork facility (apache#14594)
Also migrate the `ThreadPool` class to use the new facility. The `util::GlobalForkSafeMutex` facility is now unused, we may want to remove it in a later PR. Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent 7f067f9 commit 619b034

10 files changed

Lines changed: 586 additions & 78 deletions

File tree

cpp/src/arrow/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ set(ARROW_SRCS
192192
io/stdio.cc
193193
io/transform.cc
194194
util/async_util.cc
195+
util/atfork_internal.cc
195196
util/basic_decimal.cc
196197
util/bit_block_counter.cc
197198
util/bit_run_reader.cc

cpp/src/arrow/testing/gtest_util.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,22 @@ bool FileIsClosed(int fd) {
573573
#endif
574574
}
575575

576+
#if !defined(_WIN32)
577+
void AssertChildExit(int child_pid, int expected_exit_status) {
578+
ASSERT_GT(child_pid, 0);
579+
int child_status;
580+
int got_pid = waitpid(child_pid, &child_status, 0);
581+
ASSERT_EQ(got_pid, child_pid);
582+
if (WIFSIGNALED(child_status)) {
583+
FAIL() << "Child terminated by signal " << WTERMSIG(child_status);
584+
}
585+
if (!WIFEXITED(child_status)) {
586+
FAIL() << "Child didn't terminate normally?? Child status = " << child_status;
587+
}
588+
ASSERT_EQ(WEXITSTATUS(child_status), expected_exit_status);
589+
}
590+
#endif
591+
576592
bool LocaleExists(const char* locale) {
577593
try {
578594
std::locale loc(locale);

cpp/src/arrow/testing/gtest_util.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,11 @@ std::vector<T> IteratorToVector(Iterator<T> iterator) {
378378
ARROW_TESTING_EXPORT
379379
bool LocaleExists(const char* locale);
380380

381+
#ifndef _WIN32
382+
ARROW_TESTING_EXPORT
383+
void AssertChildExit(int child_pid, int expected_exit_status = 0);
384+
#endif
385+
381386
// A RAII-style object that switches to a new locale, and switches back
382387
// to the old locale when going out of scope. Doesn't do anything if the
383388
// new locale doesn't exist on the local machine.

cpp/src/arrow/util/CMakeLists.txt

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,7 @@ endif()
4141
add_arrow_test(utility-test
4242
SOURCES
4343
align_util_test.cc
44-
async_generator_test.cc
45-
async_util_test.cc
46-
bit_block_counter_test.cc
47-
bit_util_test.cc
44+
atfork_test.cc
4845
byte_size_test.cc
4946
cache_test.cc
5047
checked_cast_test.cc
@@ -60,7 +57,6 @@ add_arrow_test(utility-test
6057
queue_test.cc
6158
range_test.cc
6259
reflection_test.cc
63-
rle_encoding_test.cc
6460
small_vector_test.cc
6561
stl_util_test.cc
6662
string_test.cc
@@ -73,6 +69,18 @@ add_arrow_test(utility-test
7369
utf8_util_test.cc
7470
value_parsing_test.cc)
7571

72+
add_arrow_test(async-utility-test
73+
SOURCES
74+
async_generator_test.cc
75+
async_util_test.cc
76+
test_common.cc)
77+
78+
add_arrow_test(bit-utility-test
79+
SOURCES
80+
bit_block_counter_test.cc
81+
bit_util_test.cc
82+
rle_encoding_test.cc)
83+
7684
add_arrow_test(threading-utility-test
7785
SOURCES
7886
cancel_test.cc
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
#include "arrow/util/atfork_internal.h"
19+
20+
#include <algorithm>
21+
#include <atomic>
22+
#include <mutex>
23+
#include <vector>
24+
25+
#ifndef _WIN32
26+
#include <pthread.h>
27+
#endif
28+
29+
#include "arrow/util/io_util.h"
30+
#include "arrow/util/logging.h"
31+
32+
namespace arrow {
33+
namespace internal {
34+
35+
namespace {
36+
37+
struct RunningHandler {
38+
// A temporary owning copy of a handler, to make sure that a handler
39+
// that runs before fork can still run after fork.
40+
std::shared_ptr<AtForkHandler> handler;
41+
// The token returned by the before-fork handler, to pass to after-fork handlers.
42+
std::any token;
43+
44+
explicit RunningHandler(std::shared_ptr<AtForkHandler> handler)
45+
: handler(std::move(handler)) {}
46+
};
47+
48+
std::mutex g_mutex;
49+
std::vector<std::weak_ptr<AtForkHandler>> g_handlers;
50+
std::vector<RunningHandler> g_handlers_while_forking;
51+
52+
void MaintainHandlersUnlocked() {
53+
auto it = std::remove_if(
54+
g_handlers.begin(), g_handlers.end(),
55+
[](const std::weak_ptr<AtForkHandler>& ptr) { return ptr.expired(); });
56+
g_handlers.erase(it, g_handlers.end());
57+
}
58+
59+
void BeforeFork() {
60+
// Lock the mutex and keep it locked until the end of AfterForkParent(),
61+
// to avoid multiple concurrent forks and atforks.
62+
g_mutex.lock();
63+
64+
DCHECK(g_handlers_while_forking.empty()); // AfterForkParent clears it
65+
66+
for (const auto& weak_handler : g_handlers) {
67+
if (auto handler = weak_handler.lock()) {
68+
g_handlers_while_forking.emplace_back(std::move(handler));
69+
}
70+
}
71+
72+
// XXX can the handler call RegisterAtFork()?
73+
for (auto&& handler : g_handlers_while_forking) {
74+
if (handler.handler->before) {
75+
handler.token = handler.handler->before();
76+
}
77+
}
78+
}
79+
80+
void AfterForkParent() {
81+
// The mutex was locked by BeforeFork()
82+
83+
auto handlers = std::move(g_handlers_while_forking);
84+
g_handlers_while_forking.clear();
85+
// Execute handlers in reverse order
86+
for (auto it = handlers.rbegin(); it != handlers.rend(); ++it) {
87+
auto&& handler = *it;
88+
if (handler.handler->parent_after) {
89+
handler.handler->parent_after(std::move(handler.token));
90+
}
91+
}
92+
93+
g_mutex.unlock();
94+
// handlers will be destroyed here without the mutex locked, so that
95+
// any action taken by destructors might call RegisterAtFork
96+
}
97+
98+
void AfterForkChild() {
99+
// Need to reinitialize the mutex as it is probably invalid. Also, the
100+
// old mutex destructor may fail.
101+
// Fortunately, we are a single thread in the child process by now, so no
102+
// additional synchronization is needed.
103+
new (&g_mutex) std::mutex;
104+
105+
auto handlers = std::move(g_handlers_while_forking);
106+
g_handlers_while_forking.clear();
107+
// Execute handlers in reverse order
108+
for (auto it = handlers.rbegin(); it != handlers.rend(); ++it) {
109+
auto&& handler = *it;
110+
if (handler.handler->child_after) {
111+
handler.handler->child_after(std::move(handler.token));
112+
}
113+
}
114+
}
115+
116+
struct AtForkInitializer {
117+
AtForkInitializer() {
118+
#ifndef _WIN32
119+
int r = pthread_atfork(&BeforeFork, &AfterForkParent, &AfterForkChild);
120+
if (r != 0) {
121+
IOErrorFromErrno(r, "Error when calling pthread_atfork: ").Abort();
122+
}
123+
#endif
124+
}
125+
};
126+
127+
}; // namespace
128+
129+
void RegisterAtFork(std::weak_ptr<AtForkHandler> weak_handler) {
130+
static AtForkInitializer initializer;
131+
132+
std::lock_guard<std::mutex> lock(g_mutex);
133+
MaintainHandlersUnlocked();
134+
g_handlers.push_back(std::move(weak_handler));
135+
}
136+
137+
} // namespace internal
138+
} // namespace arrow
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
#pragma once
19+
20+
#include <any>
21+
#include <functional>
22+
#include <memory>
23+
#include <utility>
24+
25+
#include "arrow/util/visibility.h"
26+
27+
namespace arrow {
28+
namespace internal {
29+
30+
struct ARROW_EXPORT AtForkHandler {
31+
using CallbackBefore = std::function<std::any()>;
32+
using CallbackAfter = std::function<void(std::any)>;
33+
34+
// The before-fork callback can return an arbitrary token (wrapped in std::any)
35+
// that will passed as-is to after-fork callbacks. This can ensure that any
36+
// resource necessary for after-fork handling is kept alive.
37+
CallbackBefore before;
38+
CallbackAfter parent_after;
39+
CallbackAfter child_after;
40+
41+
AtForkHandler() = default;
42+
43+
explicit AtForkHandler(CallbackAfter child_after)
44+
: child_after(std::move(child_after)) {}
45+
46+
AtForkHandler(CallbackBefore before, CallbackAfter parent_after,
47+
CallbackAfter child_after)
48+
: before(std::move(before)),
49+
parent_after(std::move(parent_after)),
50+
child_after(std::move(child_after)) {}
51+
};
52+
53+
// Register the given at-fork handlers. Their intended lifetime should be tracked by
54+
// calling code using an owning shared_ptr.
55+
ARROW_EXPORT
56+
void RegisterAtFork(std::weak_ptr<AtForkHandler>);
57+
58+
} // namespace internal
59+
} // namespace arrow

0 commit comments

Comments
 (0)