New issue
Advanced search Search tips

Issue 869990 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 6
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

[Windows Host] Crash in remoting::TouchInjectorWin::InjectTouchEvent

Project Member Reported by joedow@chromium.org, Aug 1

Issue description

This has been a persistent crash on Windows for several releases:
00 07dff784 730e97a1 06efbe00 00000000 07dff7a8 remoting_core!remoting::TouchInjectorWin::InjectTouchEvent+0x1b [C:\b\c\b\win_clang\src\remoting\host\touch_injector_win.cc @ 190] 
01 07dff7d8 730ea647 06efbe00 07dff850 7338a392 remoting_core!remoting::SessionInputInjectorWin::Core::InjectTouchEvent+0x3f [C:\b\c\b\win_clang\src\remoting\host\win\session_input_injector.cc @ 204] 
02 07dff7e4 7338a392 06efbde8 bd0009b4 00000000 remoting_core!base::internal::Invoker<base::internal::BindState<void (remoting::SessionInputInjectorWin::Core::*)(const remoting::protocol::TouchEvent &) __attribute__((thiscall)),scoped_refptr<remoting::SessionInputInjectorWin::Core>,remoting::protocol::TouchEvent>,void ()>::Run+0x13 [C:\b\c\b\win_clang\src\base\bind_internal.h @ 665] 
03 07dff850 7327643e 73c2780c 07dff900 73c07c64 remoting_core!base::debug::TaskAnnotator::RunTask+0xe2 [C:\b\c\b\win_clang\src\base\debug\task_annotator.cc @ 101] 
04 07dff8d8 73276763 07dff900 7338dde0 b0810b0d remoting_core!base::MessageLoop::RunTask+0x1be [C:\b\c\b\win_clang\src\base\message_loop\message_loop.cc @ 423] 
05 07dff8f8 73276863 00000000 73c07d12 73c07c64 remoting_core!base::MessageLoop::DeferOrRunPendingTask+0x53 [C:\b\c\b\win_clang\src\base\message_loop\message_loop.cc @ 432] 
06 07dff9a8 73277f95 00000000 00000000 00000001 remoting_core!base::MessageLoop::DoWork+0xd3 [C:\b\c\b\win_clang\src\base\message_loop\message_loop.cc @ 476] 
07 07dff9e0 73277451 07dffa58 73c8c100 00000001 remoting_core!base::MessagePumpForIO::DoRunLoop+0x135 [C:\b\c\b\win_clang\src\base\message_loop\message_pump_win.cc @ 483] 
08 07dffa00 7327617f 07dffa58 07dffa40 07dffa20 remoting_core!base::MessagePumpWin::Run+0x41 [C:\b\c\b\win_clang\src\base\message_loop\message_pump_win.cc @ 54] 
09 07dffa10 73280eae 00000001 07dffa28 07dffb14 remoting_core!base::MessageLoop::Run+0x1f [C:\b\c\b\win_clang\src\base\message_loop\message_loop.cc @ 373] 
0a 07dffa20 730eb49b 06ef9360 07dffa20 07dffa20 remoting_core!base::RunLoop::Run+0x2e [C:\b\c\b\win_clang\src\base\run_loop.cc @ 108] 
0b 07dffb14 7328cbab 06ef9360 000002dc 000002dc remoting_core!remoting::AutoThread::ThreadMain+0x16b [C:\b\c\b\win_clang\src\remoting\base\auto_thread.cc @ 230] 
0c 07dffb38 745c8484 06ef93a0 745c8460 da54ae4d remoting_core!base::`anonymous namespace'::ThreadFunc+0xbb [C:\b\c\b\win_clang\src\base\threading\platform_thread_win.cc @ 94] 
0d 07dffb4c 778d2fea 06ef93a0 3da5784b 00000000 kernel32!BaseThreadInitThunk+0x24
0e 07dffb94 778d2fba ffffffff 778eec33 00000000 ntdll!__RtlUserThreadStart+0x2f
0f 07dffba4 00000000 7328caf0 06ef93a0 00000000 ntdll!_RtlUserThreadStart+0x1b

This is an optimized stack from a minidump so some details are not available, but what looks to be happening is that the InputInjectorWin object (apparently optimized out of the call stack) is asked to inject a touch event before it has created the TouchInjectorWin instance in its start method.  This is because there are three threads involved and several different objects are created / initialized on each of them.  This is a bit messy so some clean-up in the future is probably useful.

Either way, the issue appears to be that the touch injector instance is being accessed before creation so we should refactor it a bit so that can't happen.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 1

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

commit d7e58ca4f6a3ab85206db7b8f29943c1dc1fb41d
Author: Joe Downing <joedow@chromium.org>
Date: Wed Aug 01 20:55:21 2018

[Windows Host] Fixing a crash related to touch injection

This crash is an odd one as we only have an optimized stack trace and limited
info since it is from a mini dump.  From best I can tell, this crash is due
to a race condition in the start-up path for the desktop process.  In the dump,
I see the other capturers all being initialized on the other threads so this
is very early on.  It occurs because the start and inject methods related to
Touch are run on different threads.  I looked into sorting that out but it is
a bigger refactor than I would like to do at the moment (there are several
classes and three threads to sort out).

This fix ensures that the InputInjectorWin instance always has a valid
TouchInjectorWin object to call into, even if the touch class has not
been initialized yet (it is likely deferred due to the time and the fact
that we need to load a library from disk).

BUG= 869990 

Change-Id: Ic408c534e8ee71f4aa9e712f5ffb415ff97222c6
Reviewed-on: https://chromium-review.googlesource.com/1159168
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579932}
[modify] https://crrev.com/d7e58ca4f6a3ab85206db7b8f29943c1dc1fb41d/remoting/host/input_injector_win.cc
[modify] https://crrev.com/d7e58ca4f6a3ab85206db7b8f29943c1dc1fb41d/remoting/host/touch_injector_win.cc

Status: Fixed (was: Assigned)

Sign in to add a comment