Commit 2ad0be41 authored by morrita's avatar morrita Committed by Commit bot

Add ChannelPosix::ResetSafely() to deal with a lingering crash

The CL [1] tightened the error check too much and revealed
an existing inconsistency and resulted a production crash.
This CL makes a workaround to that crash, turning a PCHECK()
to DPCHECK().

This is the second attempt. The first one was reverted at [2]

[1] https://crrev.com/ce44fef5fd60dd2be5c587d4b084bdcd36adcee4
[2] https://crrev.com/b83c03c7b691914a054abaab13fb9131b4e0152b

BUG=449233,448245
R=agl@chromium.org,jam@chromium.org

Committed: https://crrev.com/cf3eae19b456536c5ff8dacf8800ec09f9af4bca
Cr-Commit-Position: refs/heads/master@{#313188}

Review URL: https://codereview.chromium.org/882543002

Cr-Commit-Position: refs/heads/master@{#313334}
parent ba059dd9
......@@ -189,6 +189,7 @@ ChannelPosix::ChannelPosix(const IPC::ChannelHandle& channel_handle,
waiting_connect_(true),
message_send_bytes_written_(0),
pipe_name_(channel_handle.name),
in_dtor_(false),
must_unlink_(false) {
memset(input_cmsg_buf_, 0, sizeof(input_cmsg_buf_));
if (!CreatePipe(channel_handle)) {
......@@ -200,6 +201,7 @@ ChannelPosix::ChannelPosix(const IPC::ChannelHandle& channel_handle,
}
ChannelPosix::~ChannelPosix() {
in_dtor_ = true;
Close();
}
......@@ -611,7 +613,7 @@ void ChannelPosix::ResetToAcceptingConnectionState() {
// Unregister libevent for the unix domain socket and close it.
read_watcher_.StopWatchingFileDescriptor();
write_watcher_.StopWatchingFileDescriptor();
pipe_.reset();
ResetSafely(&pipe_);
#if defined(IPC_USES_READWRITE)
fd_pipe_.reset();
remote_fd_pipe_.reset();
......@@ -1078,6 +1080,25 @@ base::ProcessId ChannelPosix::GetSelfPID() const {
return GetHelloMessageProcId();
}
void ChannelPosix::ResetSafely(base::ScopedFD* fd) {
if (!in_dtor_) {
fd->reset();
return;
}
// crbug.com/449233
// The CL [1] tightened the error check for closing FDs, but it turned
// out that there are existing cases that hit the newly added check.
// ResetSafely() is the workaround for that crash, turning it from
// from PCHECK() to DPCHECK() so that it doesn't crash in production.
// [1] https://crrev.com/ce44fef5fd60dd2be5c587d4b084bdcd36adcee4
int fd_to_close = fd->release();
if (-1 != fd_to_close) {
int rv = IGNORE_EINTR(close(fd_to_close));
DPCHECK(0 == rv);
}
}
//------------------------------------------------------------------------------
// Channel's methods
......
......@@ -202,6 +202,10 @@ class IPC_EXPORT ChannelPosix : public Channel,
// implementation!
std::vector<int> input_fds_;
void ResetSafely(base::ScopedFD* fd);
bool in_dtor_;
#if defined(OS_MACOSX)
// On OSX, sent FDs must not be closed until we get an ack.
// Keep track of sent FDs here to make sure the remote is not
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment