Skip to content

Commit

Permalink
Revert of Open MessagePumpLibevent's pipe with O_CLOEXEC (patchset ch…
Browse files Browse the repository at this point in the history
…romium#4 id:60001 of https://codereview.chromium.org/2394413002/ )

Reason for revert:
Reverting since it breaks NaCL tests.

https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests%20%28dbg%29%281%29%2832%29/builds/34211

failures:
NaClBrowserTestNonSfiMode.Irt
NaClBrowserTestNonSfiMode.Messaging

Original issue's description:
> Open MessagePumpLibevent's pipe with O_CLOEXEC
>
> This prevents the pipe's fds from being leaked into child processes.
>
> TEST=git cl try
> BUG=653930
>
> Committed: https://crrev.com/f9c8562e55b669915faf87d8d7eda32ae62bf661
> Cr-Commit-Position: refs/heads/master@{#423899}

TBR=dcheng@chromium.org,lhchavez@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=653930

Review-Url: https://codereview.chromium.org/2401863004
Cr-Commit-Position: refs/heads/master@{#423966}
  • Loading branch information
naskooskov authored and Commit bot committed Oct 7, 2016
1 parent c243409 commit c231da9
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 47 deletions.
11 changes: 0 additions & 11 deletions base/files/file_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -365,17 +365,6 @@ BASE_EXPORT int GetUniquePathNumber(const FilePath& path,
BASE_EXPORT bool SetNonBlocking(int fd);

#if defined(OS_POSIX)
// Creates a non-blocking, close-on-exec pipe.
// This creates a non-blocking pipe that is not intended to be shared with any
// child process. This will be done atomically if the operating system supports
// it. Returns true if it was able to create the pipe, otherwise false.
BASE_EXPORT bool CreateLocalNonBlockingPipe(int fds[2]);

// Sets the given |fd| to close-on-exec mode.
// Returns true if it was able to set it in the close-on-exec mode, otherwise
// false.
BASE_EXPORT bool SetCloseOnExec(int fd);

// Test that |path| can only be changed by a given user and members of
// a given set of groups.
// Specifically, test that all parts of |path| under (and including) |base|:
Expand Down
34 changes: 0 additions & 34 deletions base/files/file_util_posix.cc
Original file line number Diff line number Diff line change
Expand Up @@ -351,29 +351,6 @@ bool CopyDirectory(const FilePath& from_path,
}
#endif // !defined(OS_NACL_NONSFI)

bool CreateLocalNonBlockingPipe(int fds[2]) {
#if defined(OS_LINUX)
return pipe2(fds, O_CLOEXEC | O_NONBLOCK) == 0;
#else
int raw_fds[2];
if (pipe(raw_fds) != 0)
return false;
ScopedFD fd_out(raw_fds[0]);
ScopedFD fd_in(raw_fds[1]);
if (!SetCloseOnExec(fd_out.get()))
return false;
if (!SetCloseOnExec(fd_in.get()))
return false;
if (!SetNonBlocking(fd_out.get()))
return false;
if (!SetNonBlocking(fd_in.get()))
return false;
fds[0] = fd_out.release();
fds[1] = fd_in.release();
return true;
#endif
}

bool SetNonBlocking(int fd) {
const int flags = fcntl(fd, F_GETFL);
if (flags == -1)
Expand All @@ -385,17 +362,6 @@ bool SetNonBlocking(int fd) {
return true;
}

bool SetCloseOnExec(int fd) {
const int flags = fcntl(fd, F_GETFD);
if (flags == -1)
return false;
if (flags & FD_CLOEXEC)
return true;
if (HANDLE_EINTR(fcntl(fd, F_SETFD, flags | FD_CLOEXEC)) == -1)
return false;
return true;
}

bool PathExists(const FilePath& path) {
ThreadRestrictions::AssertIOAllowed();
#if defined(OS_ANDROID)
Expand Down
12 changes: 10 additions & 2 deletions base/message_loop/message_pump_libevent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,16 @@ void MessagePumpLibevent::ScheduleDelayedWork(

bool MessagePumpLibevent::Init() {
int fds[2];
if (!CreateLocalNonBlockingPipe(fds)) {
DPLOG(ERROR) << "pipe creation failed";
if (pipe(fds)) {
DLOG(ERROR) << "pipe() failed, errno: " << errno;
return false;
}
if (!SetNonBlocking(fds[0])) {
DLOG(ERROR) << "SetNonBlocking for pipe fd[0] failed, errno: " << errno;
return false;
}
if (!SetNonBlocking(fds[1])) {
DLOG(ERROR) << "SetNonBlocking for pipe fd[1] failed, errno: " << errno;
return false;
}
wakeup_pipe_out_ = fds[0];
Expand Down

0 comments on commit c231da9

Please sign in to comment.