Commit 1749d14a authored by xhwang@chromium.org's avatar xhwang@chromium.org

Add CHECK on file descriptor in various IPC::ChannelHandle passed in.

Regarding Chromium issues 73355, 95129, 95732, 97285, 103957 and Chromium-os issue 18437, 22372, we suspect the channel handles passed to the renderer have invalid file descriptors (fd). This is supported by the fact that using a channel handle with a valid name but an invalid fd will produce crashes with exactly the same stack trace as reported in these issues. Running out of fd in either the renderer, browser or the other process (GPU, broker, etc) could cause this to happen, but we are not sure if that's the real cause.

Adding check for the fd in various places to help investigate these issues further. We will be able to tell if invalid fd is passed in and if yes, which process generates it. Browser side check is only added for the broker case to limit the scale of bad user experience, while providing enough cases for investigation.

BUG=none
TEST=passed unit tests

Review URL: http://codereview.chromium.org/8735015

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@112647 0039d316-1c4b-4281-b951-d872f2087c98
parent 2bb95c0f
...@@ -159,6 +159,11 @@ class OpenChannelToPpapiBrokerCallback ...@@ -159,6 +159,11 @@ class OpenChannelToPpapiBrokerCallback
virtual void OnChannelOpened(base::ProcessHandle broker_process_handle, virtual void OnChannelOpened(base::ProcessHandle broker_process_handle,
const IPC::ChannelHandle& channel_handle) { const IPC::ChannelHandle& channel_handle) {
#if defined(OS_POSIX)
// Check the validity of fd for bug investigation. Remove after fixed.
// See for details: crbug.com/103957.
CHECK_NE(-1, channel_handle.socket.fd);
#endif
filter_->Send(new ViewMsg_PpapiBrokerChannelCreated(routing_id_, filter_->Send(new ViewMsg_PpapiBrokerChannelCreated(routing_id_,
request_id_, request_id_,
broker_process_handle, broker_process_handle,
......
...@@ -95,7 +95,9 @@ void GpuChannelManager::OnEstablishChannel(int renderer_id) { ...@@ -95,7 +95,9 @@ void GpuChannelManager::OnEstablishChannel(int renderer_id) {
// On POSIX, pass the renderer-side FD. Also mark it as auto-close so // On POSIX, pass the renderer-side FD. Also mark it as auto-close so
// that it gets closed after it has been sent. // that it gets closed after it has been sent.
int renderer_fd = channel->TakeRendererFileDescriptor(); int renderer_fd = channel->TakeRendererFileDescriptor();
DCHECK_NE(-1, renderer_fd); // Check the validity of |renderer_fd| for bug investigation. Replace with
// normal error handling after bug fixed. See for details: crbug.com/95732.
CHECK_NE(-1, renderer_fd);
channel_handle.socket = base::FileDescriptor(renderer_fd, true); channel_handle.socket = base::FileDescriptor(renderer_fd, true);
#endif #endif
} }
......
...@@ -121,6 +121,13 @@ base::WaitableEvent* NPChannelBase::GetModalDialogEvent( ...@@ -121,6 +121,13 @@ base::WaitableEvent* NPChannelBase::GetModalDialogEvent(
bool NPChannelBase::Init(base::MessageLoopProxy* ipc_message_loop, bool NPChannelBase::Init(base::MessageLoopProxy* ipc_message_loop,
bool create_pipe_now, bool create_pipe_now,
base::WaitableEvent* shutdown_event) { base::WaitableEvent* shutdown_event) {
#if defined(OS_POSIX)
// Check the validity of fd for bug investigation. Remove after fixed.
// See for details: crbug.com/95129, crbug.com/97285.
if (mode_ == IPC::Channel::MODE_CLIENT)
CHECK_NE(-1, channel_handle_.socket.fd);
#endif
channel_.reset(new IPC::SyncChannel( channel_.reset(new IPC::SyncChannel(
channel_handle_, mode_, this, ipc_message_loop, create_pipe_now, channel_handle_, mode_, this, ipc_message_loop, create_pipe_now,
shutdown_event)); shutdown_event));
......
...@@ -303,6 +303,9 @@ bool PpapiThread::SetupRendererChannel(base::ProcessHandle host_process_handle, ...@@ -303,6 +303,9 @@ bool PpapiThread::SetupRendererChannel(base::ProcessHandle host_process_handle,
// This ensures this process will be notified when it is closed even if a // This ensures this process will be notified when it is closed even if a
// connection is not established. // connection is not established.
handle->socket = base::FileDescriptor(dispatcher->TakeRendererFD(), true); handle->socket = base::FileDescriptor(dispatcher->TakeRendererFD(), true);
// Check the validity of fd for bug investigation. Remove after fixed.
// See for details: crbug.com/103957.
CHECK_NE(-1, handle->socket.fd);
if (handle->socket.fd == -1) if (handle->socket.fd == -1)
return false; return false;
#endif #endif
......
...@@ -527,6 +527,9 @@ class HostDispatcherWrapper ...@@ -527,6 +527,9 @@ class HostDispatcherWrapper
return false; return false;
#if defined(OS_POSIX) #if defined(OS_POSIX)
// Check the validity of fd for bug investigation. Remove after fixed.
// See for details: crbug.com/103957.
CHECK_NE(-1, channel_handle.socket.fd);
if (channel_handle.socket.fd == -1) if (channel_handle.socket.fd == -1)
return false; return false;
#endif #endif
......
...@@ -804,6 +804,12 @@ GpuChannelHost* RenderThreadImpl::EstablishGpuChannelSync( ...@@ -804,6 +804,12 @@ GpuChannelHost* RenderThreadImpl::EstablishGpuChannelSync(
return NULL; return NULL;
} }
#if defined(OS_POSIX)
// Check the validity of fd for bug investigation. Replace with normal error
// handling (see above) after bug fixed. See for details: crbug.com/95732.
CHECK_NE(-1, channel_handle.socket.fd);
#endif
gpu_channel_->set_gpu_info(gpu_info); gpu_channel_->set_gpu_info(gpu_info);
content::GetContentClient()->SetGpuInfo(gpu_info); content::GetContentClient()->SetGpuInfo(gpu_info);
......
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