Skip to content
This repository was archived by the owner on Jan 11, 2024. It is now read-only.

Commit 3e5edaf

Browse files
philwovladmos
authored andcommitted
process-wrapper: Wait for all (grand)children before exiting.
This uses Linux's PR_SET_CHILD_SUBREAPER and FreeBSD's PROC_REAP_ACQUIRE features to become an init-like process for all (grand)children spawned by process-wrapper, which allows us to a) kill them reliably and then b) wait for them reliably. Before this change, we only killed the main child, waited for it, then fired off a kill -9 on the process group, without waiting for it. This led to a race condition where Bazel would try to use or delete files that were still helt open by children of the main child and thus to bugs like bazelbuild#2371. This means we now have reliable process management on Linux, FreeBSD and Windows. Unfortunately I couldn't find any feature like this on macOS, so this is the only OS that will still have this race condition. PiperOrigin-RevId: 153817210
1 parent 15e1b9b commit 3e5edaf

File tree

10 files changed

+659
-641
lines changed

10 files changed

+659
-641
lines changed

src/main/tools/BUILD

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,28 @@
11
package(default_visibility = ["//src:__subpackages__"])
22

3+
cc_library(
4+
name = "process-tools",
5+
srcs = [
6+
"process-tools.cc",
7+
"process-tools.h",
8+
],
9+
)
10+
311
cc_binary(
412
name = "process-wrapper",
513
srcs = select({
614
"//src:windows_msvc": ["process-wrapper-windows.cc"],
715
"//conditions:default": [
8-
"process-tools.c",
9-
"process-tools.h",
10-
"process-wrapper.c",
16+
"process-wrapper.cc",
1117
],
1218
}),
13-
copts = select({
19+
linkopts = ["-lm"],
20+
deps = select({
1421
"//src:windows_msvc": [],
15-
"//conditions:default": ["-std=c99"],
22+
"//conditions:default": [
23+
":process-tools",
24+
],
1625
}),
17-
linkopts = ["-lm"],
1826
)
1927

2028
cc_binary(
@@ -45,6 +53,17 @@ cc_binary(
4553
],
4654
}),
4755
linkopts = ["-lm"],
56+
deps = select({
57+
"//src:darwin": [],
58+
"//src:darwin_x86_64": [],
59+
"//src:freebsd": [],
60+
"//src:windows": [],
61+
"//src:windows_msys": [],
62+
"//src:windows_msvc": [],
63+
"//conditions:default": [
64+
":process-tools",
65+
],
66+
}),
4867
)
4968

5069
filegroup(

src/main/tools/linux-sandbox-options.cc

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,13 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
#include "linux-sandbox-options.h"
16-
#include "linux-sandbox-utils.h"
15+
#include "src/main/tools/linux-sandbox-options.h"
1716

1817
#define DIE(args...) \
1918
{ \
2019
fprintf(stderr, __FILE__ ":" S__LINE__ ": \"" args); \
2120
fprintf(stderr, "\": "); \
22-
perror(NULL); \
21+
perror(nullptr); \
2322
exit(EXIT_FAILURE); \
2423
}
2524

@@ -39,6 +38,8 @@
3938
#include <string>
4039
#include <vector>
4140

41+
#include "src/main/tools/linux-sandbox-utils.h"
42+
4243
using std::ifstream;
4344
using std::unique_ptr;
4445
using std::vector;
@@ -199,6 +200,7 @@ static void ParseCommandLine(unique_ptr<vector<char *>> args) {
199200
if (optind < static_cast<int>(args->size())) {
200201
if (opt.args.empty()) {
201202
opt.args.assign(args->begin() + optind, args->end());
203+
opt.args.push_back(nullptr);
202204
} else {
203205
Usage(args->front(), "Merging commands not supported.");
204206
}
@@ -207,8 +209,8 @@ static void ParseCommandLine(unique_ptr<vector<char *>> args) {
207209

208210
// Expands a single argument, expanding options @filename to read in the content
209211
// of the file and add it to the list of processed arguments.
210-
unique_ptr<vector<char *>> ExpandArgument(unique_ptr<vector<char *>> expanded,
211-
char *arg) {
212+
static unique_ptr<vector<char *>> ExpandArgument(
213+
unique_ptr<vector<char *>> expanded, char *arg) {
212214
if (arg[0] == '@') {
213215
const char *filename = arg + 1; // strip off the '@'.
214216
ifstream f(filename);
@@ -236,7 +238,7 @@ unique_ptr<vector<char *>> ExpandArgument(unique_ptr<vector<char *>> expanded,
236238
// Pre-processes an argument list, expanding options @filename to read in the
237239
// content of the file and add it to the list of arguments. Stops expanding
238240
// arguments once it encounters "--".
239-
unique_ptr<vector<char *>> ExpandArguments(const vector<char *> &args) {
241+
static unique_ptr<vector<char *>> ExpandArguments(const vector<char *> &args) {
240242
unique_ptr<vector<char *>> expanded(new vector<char *>());
241243
expanded->reserve(args.size());
242244
for (auto arg = args.begin(); arg != args.end(); ++arg) {
@@ -260,6 +262,6 @@ void ParseOptions(int argc, char *argv[]) {
260262
}
261263

262264
if (opt.working_dir.empty()) {
263-
opt.working_dir = getcwd(NULL, 0);
265+
opt.working_dir = getcwd(nullptr, 0);
264266
}
265267
}

0 commit comments

Comments
 (0)