Skip to content

Commit

Permalink
process-wrapper: Wait for all (grand)children before exiting.
Browse files Browse the repository at this point in the history
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 #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
  • Loading branch information
philwo authored and vladmos committed Apr 24, 2017
1 parent 15e1b9b commit 3e5edaf
Show file tree
Hide file tree
Showing 10 changed files with 659 additions and 641 deletions.
31 changes: 25 additions & 6 deletions src/main/tools/BUILD
Original file line number Diff line number Diff line change
@@ -1,20 +1,28 @@
package(default_visibility = ["//src:__subpackages__"])

cc_library(
name = "process-tools",
srcs = [
"process-tools.cc",
"process-tools.h",
],
)

cc_binary(
name = "process-wrapper",
srcs = select({
"//src:windows_msvc": ["process-wrapper-windows.cc"],
"//conditions:default": [
"process-tools.c",
"process-tools.h",
"process-wrapper.c",
"process-wrapper.cc",
],
}),
copts = select({
linkopts = ["-lm"],
deps = select({
"//src:windows_msvc": [],
"//conditions:default": ["-std=c99"],
"//conditions:default": [
":process-tools",
],
}),
linkopts = ["-lm"],
)

cc_binary(
Expand Down Expand Up @@ -45,6 +53,17 @@ cc_binary(
],
}),
linkopts = ["-lm"],
deps = select({
"//src:darwin": [],
"//src:darwin_x86_64": [],
"//src:freebsd": [],
"//src:windows": [],
"//src:windows_msys": [],
"//src:windows_msvc": [],
"//conditions:default": [
":process-tools",
],
}),
)

filegroup(
Expand Down
16 changes: 9 additions & 7 deletions src/main/tools/linux-sandbox-options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#include "linux-sandbox-options.h"
#include "linux-sandbox-utils.h"
#include "src/main/tools/linux-sandbox-options.h"

#define DIE(args...) \
{ \
fprintf(stderr, __FILE__ ":" S__LINE__ ": \"" args); \
fprintf(stderr, "\": "); \
perror(NULL); \
perror(nullptr); \
exit(EXIT_FAILURE); \
}

Expand All @@ -39,6 +38,8 @@
#include <string>
#include <vector>

#include "src/main/tools/linux-sandbox-utils.h"

using std::ifstream;
using std::unique_ptr;
using std::vector;
Expand Down Expand Up @@ -199,6 +200,7 @@ static void ParseCommandLine(unique_ptr<vector<char *>> args) {
if (optind < static_cast<int>(args->size())) {
if (opt.args.empty()) {
opt.args.assign(args->begin() + optind, args->end());
opt.args.push_back(nullptr);
} else {
Usage(args->front(), "Merging commands not supported.");
}
Expand All @@ -207,8 +209,8 @@ static void ParseCommandLine(unique_ptr<vector<char *>> args) {

// Expands a single argument, expanding options @filename to read in the content
// of the file and add it to the list of processed arguments.
unique_ptr<vector<char *>> ExpandArgument(unique_ptr<vector<char *>> expanded,
char *arg) {
static unique_ptr<vector<char *>> ExpandArgument(
unique_ptr<vector<char *>> expanded, char *arg) {
if (arg[0] == '@') {
const char *filename = arg + 1; // strip off the '@'.
ifstream f(filename);
Expand Down Expand Up @@ -236,7 +238,7 @@ unique_ptr<vector<char *>> ExpandArgument(unique_ptr<vector<char *>> expanded,
// Pre-processes an argument list, expanding options @filename to read in the
// content of the file and add it to the list of arguments. Stops expanding
// arguments once it encounters "--".
unique_ptr<vector<char *>> ExpandArguments(const vector<char *> &args) {
static unique_ptr<vector<char *>> ExpandArguments(const vector<char *> &args) {
unique_ptr<vector<char *>> expanded(new vector<char *>());
expanded->reserve(args.size());
for (auto arg = args.begin(); arg != args.end(); ++arg) {
Expand All @@ -260,6 +262,6 @@ void ParseOptions(int argc, char *argv[]) {
}

if (opt.working_dir.empty()) {
opt.working_dir = getcwd(NULL, 0);
opt.working_dir = getcwd(nullptr, 0);
}
}
Loading

0 comments on commit 3e5edaf

Please sign in to comment.