Clean up process spawning.

Pass environment as argument to execvpe and use _exit instead of exit
(so fork/vfork can be used interchangeable).

BUG=
R=sgjesse@google.com

Review URL: https://codereview.chromium.org//1156313004.
This commit is contained in:
Anders Johnsen 2015-06-10 10:42:54 +02:00
parent 6a85394310
commit bbdc57ebf6
4 changed files with 83 additions and 124 deletions

View file

@ -24,9 +24,6 @@
#include "platform/signal_blocker.h"
extern char **environ;
namespace dart {
namespace bin {
@ -154,7 +151,7 @@ class ExitCodeHandler {
// Fork to wake up waitpid.
if (TEMP_FAILURE_RETRY(fork()) == 0) {
exit(0);
_exit(0);
}
monitor_->Notify();
@ -414,7 +411,7 @@ class ProcessStarter {
int bytes_read = FDUtils::ReadFromBlocking(read_in_[0], &msg, sizeof(msg));
if (bytes_read != sizeof(msg)) {
perror("Failed receiving notification message");
exit(1);
_exit(1);
}
if (mode_ == kNormal) {
ExecProcess();
@ -443,12 +440,14 @@ class ProcessStarter {
}
if (program_environment_ != NULL) {
environ = program_environment_;
VOID_TEMP_FAILURE_RETRY(
execvpe(path_, const_cast<char* const*>(program_arguments_),
program_environment_));
} else {
VOID_TEMP_FAILURE_RETRY(
execvp(path_, const_cast<char* const*>(program_arguments_)));
}
VOID_TEMP_FAILURE_RETRY(
execvp(path_, const_cast<char* const*>(program_arguments_)));
ReportChildError();
}
@ -500,13 +499,13 @@ class ProcessStarter {
execvp(path_, const_cast<char* const*>(program_arguments_)));
ReportChildError();
} else {
// Exit the intermeiate process.
exit(0);
// Exit the intermediate process.
_exit(0);
}
}
} else {
// Exit the intermeiate process.
exit(0);
// Exit the intermediate process.
_exit(0);
}
}
@ -536,7 +535,7 @@ class ProcessStarter {
FDUtils::ReadFromBlocking(
exec_control_[0], &child_errno, sizeof(child_errno));
if (bytes_read == sizeof(child_errno)) {
ReadChildError();
SetOSErrorMessage(child_errno);
return child_errno;
} else if (bytes_read == -1) {
return errno;
@ -560,7 +559,7 @@ class ProcessStarter {
} else if (bytes_read == 2 * sizeof(int)) {
*pid = result[0];
child_errno = result[1];
ReadChildError();
SetOSErrorMessage(child_errno);
return child_errno;
} else if (bytes_read == -1) {
return errno;
@ -650,21 +649,12 @@ class ProcessStarter {
void ReportChildError() {
// In the case of failure in the child process write the errno and
// the OS error message to the exec control pipe and exit.
// In the case of failure in the child process write the errno to the exec
// control pipe and exit.
int child_errno = errno;
const int kBufferSize = 1024;
char os_error_message[kBufferSize];
strerror_r(errno, os_error_message, kBufferSize);
int bytes_written =
FDUtils::WriteToBlocking(
exec_control_[1], &child_errno, sizeof(child_errno));
if (bytes_written == sizeof(child_errno)) {
FDUtils::WriteToBlocking(
exec_control_[1], os_error_message, strlen(os_error_message) + 1);
}
VOID_TEMP_FAILURE_RETRY(close(exec_control_[1]));
exit(1);
FDUtils::WriteToBlocking(
exec_control_[1], &child_errno, sizeof(child_errno));
_exit(1);
}
@ -678,16 +668,16 @@ class ProcessStarter {
}
void ReadChildError() {
void SetOSErrorMessage(int child_errno) {
const int kMaxMessageSize = 256;
char* message = static_cast<char*>(malloc(kMaxMessageSize));
if (message != NULL) {
FDUtils::ReadFromBlocking(exec_control_[0], message, kMaxMessageSize);
message[kMaxMessageSize - 1] = '\0';
char* message = static_cast<char*>(calloc(kMaxMessageSize, 0));
char* os_error_message = strerror_r(
child_errno, message, kMaxMessageSize - 1);
if (message == os_error_message) {
*os_error_message_ = message;
} else {
// Could not get error message. It will be NULL.
ASSERT(*os_error_message_ == NULL);
free(message);
*os_error_message_ = strdup(os_error_message);
}
}

View file

@ -23,9 +23,6 @@
#include "bin/thread.h"
extern char **environ;
namespace dart {
namespace bin {
@ -153,7 +150,7 @@ class ExitCodeHandler {
// Fork to wake up waitpid.
if (TEMP_FAILURE_RETRY(fork()) == 0) {
exit(0);
_exit(0);
}
monitor_->Notify();
@ -413,7 +410,7 @@ class ProcessStarter {
int bytes_read = FDUtils::ReadFromBlocking(read_in_[0], &msg, sizeof(msg));
if (bytes_read != sizeof(msg)) {
perror("Failed receiving notification message");
exit(1);
_exit(1);
}
if (mode_ == kNormal) {
ExecProcess();
@ -442,12 +439,14 @@ class ProcessStarter {
}
if (program_environment_ != NULL) {
environ = program_environment_;
VOID_TEMP_FAILURE_RETRY(
execvpe(path_, const_cast<char* const*>(program_arguments_),
program_environment_));
} else {
VOID_TEMP_FAILURE_RETRY(
execvp(path_, const_cast<char* const*>(program_arguments_)));
}
VOID_TEMP_FAILURE_RETRY(
execvp(path_, const_cast<char* const*>(program_arguments_)));
ReportChildError();
}
@ -499,13 +498,13 @@ class ProcessStarter {
execvp(path_, const_cast<char* const*>(program_arguments_)));
ReportChildError();
} else {
// Exit the intermeiate process.
exit(0);
// Exit the intermediate process.
_exit(0);
}
}
} else {
// Exit the intermeiate process.
exit(0);
// Exit the intermediate process.
_exit(0);
}
}
@ -535,7 +534,7 @@ class ProcessStarter {
FDUtils::ReadFromBlocking(
exec_control_[0], &child_errno, sizeof(child_errno));
if (bytes_read == sizeof(child_errno)) {
ReadChildError();
SetOSErrorMessage(child_errno);
return child_errno;
} else if (bytes_read == -1) {
return errno;
@ -559,7 +558,7 @@ class ProcessStarter {
} else if (bytes_read == 2 * sizeof(int)) {
*pid = result[0];
child_errno = result[1];
ReadChildError();
SetOSErrorMessage(child_errno);
return child_errno;
} else if (bytes_read == -1) {
return errno;
@ -648,21 +647,12 @@ class ProcessStarter {
void ReportChildError() {
// In the case of failure in the child process write the errno and
// the OS error message to the exec control pipe and exit.
// In the case of failure in the child process write the errno to the exec
// control pipe and exit.
int child_errno = errno;
const int kBufferSize = 1024;
char error_buf[kBufferSize];
char* os_error_message = strerror_r(errno, error_buf, kBufferSize);
int bytes_written =
FDUtils::WriteToBlocking(
exec_control_[1], &child_errno, sizeof(child_errno));
if (bytes_written == sizeof(child_errno)) {
FDUtils::WriteToBlocking(
exec_control_[1], os_error_message, strlen(os_error_message) + 1);
}
VOID_TEMP_FAILURE_RETRY(close(exec_control_[1]));
exit(1);
FDUtils::WriteToBlocking(
exec_control_[1], &child_errno, sizeof(child_errno));
_exit(1);
}
@ -676,16 +666,16 @@ class ProcessStarter {
}
void ReadChildError() {
void SetOSErrorMessage(int child_errno) {
const int kMaxMessageSize = 256;
char* message = static_cast<char*>(malloc(kMaxMessageSize));
if (message != NULL) {
FDUtils::ReadFromBlocking(exec_control_[0], message, kMaxMessageSize);
message[kMaxMessageSize - 1] = '\0';
char* message = static_cast<char*>(calloc(kMaxMessageSize, 0));
char* os_error_message = strerror_r(
child_errno, message, kMaxMessageSize - 1);
if (message == os_error_message) {
*os_error_message_ = message;
} else {
// Could not get error message. It will be NULL.
ASSERT(*os_error_message_ == NULL);
free(message);
*os_error_message_ = strdup(os_error_message);
}
}

View file

@ -25,7 +25,6 @@
#include "platform/signal_blocker.h"
namespace dart {
namespace bin {
@ -153,7 +152,7 @@ class ExitCodeHandler {
// Fork to wake up waitpid.
if (TEMP_FAILURE_RETRY(fork()) == 0) {
exit(0);
_exit(0);
}
monitor_->Notify();
@ -421,7 +420,7 @@ class ProcessStarter {
int bytes_read = FDUtils::ReadFromBlocking(read_in_[0], &msg, sizeof(msg));
if (bytes_read != sizeof(msg)) {
perror("Failed receiving notification message");
exit(1);
_exit(1);
}
if (mode_ == kNormal) {
ExecProcess();
@ -450,15 +449,14 @@ class ProcessStarter {
}
if (program_environment_ != NULL) {
// On MacOS you have to do a bit of magic to get to the
// environment strings.
char*** environ = _NSGetEnviron();
*environ = program_environment_;
VOID_TEMP_FAILURE_RETRY(
execvpe(path_, const_cast<char* const*>(program_arguments_),
program_environment_));
} else {
VOID_TEMP_FAILURE_RETRY(
execvp(path_, const_cast<char* const*>(program_arguments_)));
}
VOID_TEMP_FAILURE_RETRY(
execvp(path_, const_cast<char* const*>(program_arguments_)));
ReportChildError();
}
@ -510,13 +508,13 @@ class ProcessStarter {
execvp(path_, const_cast<char* const*>(program_arguments_)));
ReportChildError();
} else {
// Exit the intermeiate process.
exit(0);
// Exit the intermediate process.
_exit(0);
}
}
} else {
// Exit the intermeiate process.
exit(0);
// Exit the intermediate process.
_exit(0);
}
}
@ -548,7 +546,7 @@ class ProcessStarter {
FDUtils::ReadFromBlocking(
exec_control_[0], &child_errno, sizeof(child_errno));
if (bytes_read == sizeof(child_errno)) {
ReadChildError();
SetOSErrorMessage(child_errno);
return child_errno;
} else if (bytes_read == -1) {
return errno;
@ -572,7 +570,7 @@ class ProcessStarter {
} else if (bytes_read == 2 * sizeof(int)) {
*pid = result[0];
child_errno = result[1];
ReadChildError();
SetOSErrorMessage(child_errno);
return child_errno;
} else if (bytes_read == -1) {
return errno;
@ -662,21 +660,12 @@ class ProcessStarter {
void ReportChildError() {
// In the case of failure in the child process write the errno and
// the OS error message to the exec control pipe and exit.
// In the case of failure in the child process write the errno to the exec
// control pipe and exit.
int child_errno = errno;
const int kBufferSize = 1024;
char os_error_message[kBufferSize];
strerror_r(errno, os_error_message, kBufferSize);
int bytes_written =
FDUtils::WriteToBlocking(
exec_control_[1], &child_errno, sizeof(child_errno));
if (bytes_written == sizeof(child_errno)) {
FDUtils::WriteToBlocking(
exec_control_[1], os_error_message, strlen(os_error_message) + 1);
}
VOID_TEMP_FAILURE_RETRY(close(exec_control_[1]));
exit(1);
FDUtils::WriteToBlocking(
exec_control_[1], &child_errno, sizeof(child_errno));
_exit(1);
}
@ -690,16 +679,16 @@ class ProcessStarter {
}
void ReadChildError() {
void SetOSErrorMessage(int child_errno) {
const int kMaxMessageSize = 256;
char* message = static_cast<char*>(malloc(kMaxMessageSize));
if (message != NULL) {
FDUtils::ReadFromBlocking(exec_control_[0], message, kMaxMessageSize);
message[kMaxMessageSize - 1] = '\0';
char* message = static_cast<char*>(calloc(kMaxMessageSize, 0));
char* os_error_message = strerror_r(
child_errno, message, kMaxMessageSize - 1);
if (message == os_error_message) {
*os_error_message_ = message;
} else {
// Could not get error message. It will be NULL.
ASSERT(*os_error_message_ == NULL);
free(message);
*os_error_message_ = strdup(os_error_message);
}
}

View file

@ -43,20 +43,10 @@ void testDartExecShouldNotBeInCurrentDir() {
expectEquals(FileSystemEntityType.NOT_FOUND, type);
}
void testShouldFailOutsidePath() {
var threw = false;
try {
Process.runSync(platformExeName, ['--version'],
includeParentEnvironment: false,
environment: {_SCRIPT_KEY: 'yes', 'PATH': ''});
} catch (_) {
threw = true;
}
if (!threw) {
throw 'Expected running the dart executable "$platformExeName" without'
' the parent environment or path to fail.';
}
void testShouldSucceedWithEmptyPathEnvironment() {
Process.runSync(platformExeName, ['--version'],
includeParentEnvironment: false,
environment: {_SCRIPT_KEY: 'yes', 'PATH': ''});
}
void testShouldSucceedWithSourcePlatformExecutable() {
@ -149,5 +139,5 @@ void main() {
testPathToSDKDir(); /// 03: ok
withTempDir(testPathPointsToSymLinkedSDKPath); /// 04: ok
withTempDir(testPathToDirWithExeSymLinked); /// 05: ok
testShouldFailOutsidePath(); /// 06: ok
testShouldSucceedWithEmptyPathEnvironment(); /// 06: ok
}