Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(269)

Unified Diff: ipc/ipc_channel_posix.cc

Issue 882543002: Add ChannelPosix::ResetSafely() to deal with a lingering crash (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Landing Created 5 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « ipc/ipc_channel_posix.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: ipc/ipc_channel_posix.cc
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index c347b15c9e55ab4b5714df9f14d813f785baa89e..1989669d1363e6c770c281b093ba9107852be793 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -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
« no previous file with comments | « ipc/ipc_channel_posix.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698