New issue
Advanced search Search tips

Issue 812261 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

Crash in remoting::IqSender::SendIq

Project Member Reported by joedow@chromium.org, Feb 14 2018

Issue description

We are seeing this crash for M65 hosts

0x5693feb2	(remoting_core.dll -iq_sender.cc:53 )	remoting::IqSender::SendIq(std::unique_ptr<buzz::XmlElement,std::default_delete<buzz::XmlElement> >,base::RepeatingCallback<void > const &)
0x5693ffde	(remoting_core.dll -iq_sender.cc:71 )	remoting::IqSender::SendIq(std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,std::basic_string<char,std::char_traits<char>,std::allocator<char> > const &,std::unique_ptr<buzz::XmlElement,std::default_delete<buzz::XmlElement> >,base::RepeatingCallback<void > const &)
0x569a96df	(remoting_core.dll -heartbeat_sender.cc:158 )	remoting::HeartbeatSender::SendHeartbeat()
0x564e661a	(remoting_core.dll -timer.cc:50 )	base::BaseTimerTaskInternal::Run()
0x56531e9a	(remoting_core.dll -task_annotator.cc:55 )	base::debug::TaskAnnotator::RunTask(char const *,base::PendingTask *)
0x564d0bfd	(remoting_core.dll -message_loop.cc:399 )	base::MessageLoop::RunTask(base::PendingTask *)
0x564d0dd1	(remoting_core.dll -message_loop.cc:411 )	base::MessageLoop::DeferOrRunPendingTask(base::PendingTask)
0x564d10dd	(remoting_core.dll -message_loop.cc:495 )	base::MessageLoop::DoDelayedWork(base::TimeTicks *)
0x56511f88	(remoting_core.dll -message_pump_win.cc:483 )	base::MessagePumpForIO::DoRunLoop()
0x56511734	(remoting_core.dll -message_pump_win.cc:56 )	base::MessagePumpWin::Run(base::MessagePump::Delegate *)
0x564ce987	(remoting_core.dll -run_loop.cc:130 )	base::RunLoop::Run()
0x563ede93	(remoting_core.dll -auto_thread.cc:227 )	remoting::AutoThread::ThreadMain()
0x564e20da	(remoting_core.dll -platform_thread_win.cc:91 )	base::`anonymous namespace'::ThreadFunc
0x74d962c3	(kernel32.dll + 0x000162c3 )	BaseThreadInitThunk
0x77b60f78	(ntdll.dll + 0x00060f78 )	__RtlUserThreadStart
0x77b60f43	(ntdll.dll + 0x00060f43 )	_RtlUserThreadStart

The rollout has just started so there aren't too many instances on this (It is #5 on the list) but there are more unique instances than the other crashes.

I don't see this error in M63 so I think this is because we no longer immediately crash when failing to send a heartbeat (fixed in M65).
 
Labels: M-66
Owner: joedow@chromium.org
Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8a9e1de78e602a5f4cd95b16c0aa43e9ee47f813

commit 8a9e1de78e602a5f4cd95b16c0aa43e9ee47f813
Author: Joe Downing <joedow@chromium.org>
Date: Sat Feb 17 02:58:17 2018

Fixing a crash in IqSender

We are seeing a crash in M65 where the Heartbeat sender attempts to
resend a heartbeat after a timeout and the iq_sender_ member is null.
The SendHeartbeat call is being triggered via a callback and that only
occurs when the previous heartbeat times out.  This is odd because the
code which resets the iq_sender_ member also stops the timer.

After some digging, I *think* what is happening is that timer_.Stop()
does not clear any pending tasks if they have already been posted.
In this case the timer is stopped but the posted task runs shortly
afterwards with iq_sender_ being null and it crashes.

I only have access to the minidump for the crash so this is the most
likely scenario I can think of.

To address this, I have done two things:
- I've updated the Stop calls to use AbandonAndStop (this will clear
  any posted tasks)
- I've added a null check for iq_sender_

The second step may not be required but I don't want to find out in
M67 that the Abandon and stop change was not enough : P

Note: I also cleaned up some old coding conventions around callback
use in the file.  If they are too distracting I can make a follow-up
CL.

BUG= 812261 

Change-Id: I742d6ce27bc4a83ea42520c5cd8a14e911fc90e2
Reviewed-on: https://chromium-review.googlesource.com/924590
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537517}
[modify] https://crrev.com/8a9e1de78e602a5f4cd95b16c0aa43e9ee47f813/remoting/host/heartbeat_sender.cc

Comment 3 by joedow@chromium.org, Feb 20 2018

Status: Fixed (was: Assigned)

Sign in to add a comment