Skip to content

Commit bbdc57e

Browse files
author
Anders Johnsen
committed
Clean up process spawning.
Pass environment as argument to execvpe and use _exit instead of exit (so fork/vfork can be used interchangeable). BUG= [email protected] Review URL: https://codereview.chromium.org//1156313004.
1 parent 6a85394 commit bbdc57e

File tree

4 files changed

+83
-124
lines changed

4 files changed

+83
-124
lines changed

‎runtime/bin/process_android.cc‎

Lines changed: 26 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@
2424
#include "platform/signal_blocker.h"
2525

2626

27-
extern char **environ;
28-
29-
3027
namespace dart {
3128
namespace bin {
3229

@@ -154,7 +151,7 @@ class ExitCodeHandler {
154151

155152
// Fork to wake up waitpid.
156153
if (TEMP_FAILURE_RETRY(fork()) == 0) {
157-
exit(0);
154+
_exit(0);
158155
}
159156

160157
monitor_->Notify();
@@ -414,7 +411,7 @@ class ProcessStarter {
414411
int bytes_read = FDUtils::ReadFromBlocking(read_in_[0], &msg, sizeof(msg));
415412
if (bytes_read != sizeof(msg)) {
416413
perror("Failed receiving notification message");
417-
exit(1);
414+
_exit(1);
418415
}
419416
if (mode_ == kNormal) {
420417
ExecProcess();
@@ -443,12 +440,14 @@ class ProcessStarter {
443440
}
444441

445442
if (program_environment_ != NULL) {
446-
environ = program_environment_;
443+
VOID_TEMP_FAILURE_RETRY(
444+
execvpe(path_, const_cast<char* const*>(program_arguments_),
445+
program_environment_));
446+
} else {
447+
VOID_TEMP_FAILURE_RETRY(
448+
execvp(path_, const_cast<char* const*>(program_arguments_)));
447449
}
448450

449-
VOID_TEMP_FAILURE_RETRY(
450-
execvp(path_, const_cast<char* const*>(program_arguments_)));
451-
452451
ReportChildError();
453452
}
454453

@@ -500,13 +499,13 @@ class ProcessStarter {
500499
execvp(path_, const_cast<char* const*>(program_arguments_)));
501500
ReportChildError();
502501
} else {
503-
// Exit the intermeiate process.
504-
exit(0);
502+
// Exit the intermediate process.
503+
_exit(0);
505504
}
506505
}
507506
} else {
508-
// Exit the intermeiate process.
509-
exit(0);
507+
// Exit the intermediate process.
508+
_exit(0);
510509
}
511510
}
512511

@@ -536,7 +535,7 @@ class ProcessStarter {
536535
FDUtils::ReadFromBlocking(
537536
exec_control_[0], &child_errno, sizeof(child_errno));
538537
if (bytes_read == sizeof(child_errno)) {
539-
ReadChildError();
538+
SetOSErrorMessage(child_errno);
540539
return child_errno;
541540
} else if (bytes_read == -1) {
542541
return errno;
@@ -560,7 +559,7 @@ class ProcessStarter {
560559
} else if (bytes_read == 2 * sizeof(int)) {
561560
*pid = result[0];
562561
child_errno = result[1];
563-
ReadChildError();
562+
SetOSErrorMessage(child_errno);
564563
return child_errno;
565564
} else if (bytes_read == -1) {
566565
return errno;
@@ -650,21 +649,12 @@ class ProcessStarter {
650649

651650

652651
void ReportChildError() {
653-
// In the case of failure in the child process write the errno and
654-
// the OS error message to the exec control pipe and exit.
652+
// In the case of failure in the child process write the errno to the exec
653+
// control pipe and exit.
655654
int child_errno = errno;
656-
const int kBufferSize = 1024;
657-
char os_error_message[kBufferSize];
658-
strerror_r(errno, os_error_message, kBufferSize);
659-
int bytes_written =
660-
FDUtils::WriteToBlocking(
661-
exec_control_[1], &child_errno, sizeof(child_errno));
662-
if (bytes_written == sizeof(child_errno)) {
663-
FDUtils::WriteToBlocking(
664-
exec_control_[1], os_error_message, strlen(os_error_message) + 1);
665-
}
666-
VOID_TEMP_FAILURE_RETRY(close(exec_control_[1]));
667-
exit(1);
655+
FDUtils::WriteToBlocking(
656+
exec_control_[1], &child_errno, sizeof(child_errno));
657+
_exit(1);
668658
}
669659

670660

@@ -678,16 +668,16 @@ class ProcessStarter {
678668
}
679669

680670

681-
void ReadChildError() {
671+
void SetOSErrorMessage(int child_errno) {
682672
const int kMaxMessageSize = 256;
683-
char* message = static_cast<char*>(malloc(kMaxMessageSize));
684-
if (message != NULL) {
685-
FDUtils::ReadFromBlocking(exec_control_[0], message, kMaxMessageSize);
686-
message[kMaxMessageSize - 1] = '\0';
673+
char* message = static_cast<char*>(calloc(kMaxMessageSize, 0));
674+
char* os_error_message = strerror_r(
675+
child_errno, message, kMaxMessageSize - 1);
676+
if (message == os_error_message) {
687677
*os_error_message_ = message;
688678
} else {
689-
// Could not get error message. It will be NULL.
690-
ASSERT(*os_error_message_ == NULL);
679+
free(message);
680+
*os_error_message_ = strdup(os_error_message);
691681
}
692682
}
693683

‎runtime/bin/process_linux.cc‎

Lines changed: 26 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@
2323
#include "bin/thread.h"
2424

2525

26-
extern char **environ;
27-
28-
2926
namespace dart {
3027
namespace bin {
3128

@@ -153,7 +150,7 @@ class ExitCodeHandler {
153150

154151
// Fork to wake up waitpid.
155152
if (TEMP_FAILURE_RETRY(fork()) == 0) {
156-
exit(0);
153+
_exit(0);
157154
}
158155

159156
monitor_->Notify();
@@ -413,7 +410,7 @@ class ProcessStarter {
413410
int bytes_read = FDUtils::ReadFromBlocking(read_in_[0], &msg, sizeof(msg));
414411
if (bytes_read != sizeof(msg)) {
415412
perror("Failed receiving notification message");
416-
exit(1);
413+
_exit(1);
417414
}
418415
if (mode_ == kNormal) {
419416
ExecProcess();
@@ -442,12 +439,14 @@ class ProcessStarter {
442439
}
443440

444441
if (program_environment_ != NULL) {
445-
environ = program_environment_;
442+
VOID_TEMP_FAILURE_RETRY(
443+
execvpe(path_, const_cast<char* const*>(program_arguments_),
444+
program_environment_));
445+
} else {
446+
VOID_TEMP_FAILURE_RETRY(
447+
execvp(path_, const_cast<char* const*>(program_arguments_)));
446448
}
447449

448-
VOID_TEMP_FAILURE_RETRY(
449-
execvp(path_, const_cast<char* const*>(program_arguments_)));
450-
451450
ReportChildError();
452451
}
453452

@@ -499,13 +498,13 @@ class ProcessStarter {
499498
execvp(path_, const_cast<char* const*>(program_arguments_)));
500499
ReportChildError();
501500
} else {
502-
// Exit the intermeiate process.
503-
exit(0);
501+
// Exit the intermediate process.
502+
_exit(0);
504503
}
505504
}
506505
} else {
507-
// Exit the intermeiate process.
508-
exit(0);
506+
// Exit the intermediate process.
507+
_exit(0);
509508
}
510509
}
511510

@@ -535,7 +534,7 @@ class ProcessStarter {
535534
FDUtils::ReadFromBlocking(
536535
exec_control_[0], &child_errno, sizeof(child_errno));
537536
if (bytes_read == sizeof(child_errno)) {
538-
ReadChildError();
537+
SetOSErrorMessage(child_errno);
539538
return child_errno;
540539
} else if (bytes_read == -1) {
541540
return errno;
@@ -559,7 +558,7 @@ class ProcessStarter {
559558
} else if (bytes_read == 2 * sizeof(int)) {
560559
*pid = result[0];
561560
child_errno = result[1];
562-
ReadChildError();
561+
SetOSErrorMessage(child_errno);
563562
return child_errno;
564563
} else if (bytes_read == -1) {
565564
return errno;
@@ -648,21 +647,12 @@ class ProcessStarter {
648647

649648

650649
void ReportChildError() {
651-
// In the case of failure in the child process write the errno and
652-
// the OS error message to the exec control pipe and exit.
650+
// In the case of failure in the child process write the errno to the exec
651+
// control pipe and exit.
653652
int child_errno = errno;
654-
const int kBufferSize = 1024;
655-
char error_buf[kBufferSize];
656-
char* os_error_message = strerror_r(errno, error_buf, kBufferSize);
657-
int bytes_written =
658-
FDUtils::WriteToBlocking(
659-
exec_control_[1], &child_errno, sizeof(child_errno));
660-
if (bytes_written == sizeof(child_errno)) {
661-
FDUtils::WriteToBlocking(
662-
exec_control_[1], os_error_message, strlen(os_error_message) + 1);
663-
}
664-
VOID_TEMP_FAILURE_RETRY(close(exec_control_[1]));
665-
exit(1);
653+
FDUtils::WriteToBlocking(
654+
exec_control_[1], &child_errno, sizeof(child_errno));
655+
_exit(1);
666656
}
667657

668658

@@ -676,16 +666,16 @@ class ProcessStarter {
676666
}
677667

678668

679-
void ReadChildError() {
669+
void SetOSErrorMessage(int child_errno) {
680670
const int kMaxMessageSize = 256;
681-
char* message = static_cast<char*>(malloc(kMaxMessageSize));
682-
if (message != NULL) {
683-
FDUtils::ReadFromBlocking(exec_control_[0], message, kMaxMessageSize);
684-
message[kMaxMessageSize - 1] = '\0';
671+
char* message = static_cast<char*>(calloc(kMaxMessageSize, 0));
672+
char* os_error_message = strerror_r(
673+
child_errno, message, kMaxMessageSize - 1);
674+
if (message == os_error_message) {
685675
*os_error_message_ = message;
686676
} else {
687-
// Could not get error message. It will be NULL.
688-
ASSERT(*os_error_message_ == NULL);
677+
free(message);
678+
*os_error_message_ = strdup(os_error_message);
689679
}
690680
}
691681

0 commit comments

Comments
 (0)