Issue 124091 in chromium: Chromoting: the host plugin calls a pure virtual method at remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3

[chro...@googlecode.com]

Status: Assigned
Owner: alexe...@chromium.org
Labels: Type-Bug Pri-1 Area-Internals OS-Windows Mstone-20
Feature-Chromoting

New issue 124091 by alexe...@chromium.org: Chromoting: the host plugin
calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3
http://code.google.com/p/chromium/issues/detail?id=124091

Version: 1.4.1104.0
OS: Windows

What steps will reproduce the problem?
1. Enable me2me host.

Please use labels and text to provide additional information.

0:005> kL
ChildEBP RetAddr
036ef584 746a0bdd ntdll!NtWaitForMultipleObjects+0x15
036ef620 74851a2c KERNELBASE!WaitForMultipleObjectsEx+0x100
036ef668 74854208 kernel32!WaitForMultipleObjectsExImplementation+0xe0
036ef684 748780a4 kernel32!WaitForMultipleObjects+0x18
036ef6f0 74877f63 kernel32!WerpReportFaultInternal+0x186
036ef704 74877858 kernel32!WerpReportFault+0x70
036ef714 748777d7 kernel32!BasepReportFault+0x20
036ef7a0 627677ca kernel32!UnhandledExceptionFilter+0x1af
036efad8 626c1663 remoting_host_plugin!abort+0x10f
036efaf4 626c0934 remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3
036efb50 626c51ff remoting_host_plugin!MessageLoop::PostDelayedTask+0x44
036efb88 626c5108
remoting_host_plugin!base::Timer::PostNewScheduledTask+0x8f
036efbac 626e4085 remoting_host_plugin!base::Timer::Reset+0x28
036efc5c 626c530f
remoting_host_plugin!remoting::DaemonComInstallerWin::PollInstallationStatus+0x8e
036efc80 626c4ead remoting_host_plugin!base::Timer::RunScheduledTask+0x8f
036efc8c 626c0fdb remoting_host_plugin!base::BaseTimerTaskInternal::Run+0x1d
036efcf0 626c1c18 remoting_host_plugin!MessageLoop::RunTask+0x1db
036efd5c 626d6b73 remoting_host_plugin!MessageLoop::DoDelayedWork+0x158
036efd90 626d64ee
remoting_host_plugin!base::MessagePumpForUI::DoRunLoop+0x73
036efdb0 626c0b49 remoting_host_plugin!base::MessagePumpWin::Run+0x3e
036efdd4 626bab4b remoting_host_plugin!MessageLoop::Run+0x99
036efddc 626babe3 remoting_host_plugin!base::Thread::Run+0xb
036eff34 626baf0b remoting_host_plugin!base::Thread::ThreadMain+0x93
036eff40 7485339a remoting_host_plugin!base::`anonymous
namespace'::ThreadFunc+0x1b
036eff4c 76ef9ef2 kernel32!BaseThreadInitThunk+0xe
036eff8c 76ef9ec5 ntdll!__RtlUserThreadStart+0x70
036effa4 00000000 ntdll!_RtlUserThreadStart+0x1b

[chro...@googlecode.com]

Updates:
Status: Started

Comment #1 on issue 124091 by alexe...@chromium.org: Chromoting: the host

plugin calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3
http://code.google.com/p/chromium/issues/detail?id=124091

This is caused by a clash between window classes registered for
base::MessagePumpForUI by chrome and remoting_host_plugin. See the stack
below and note that on response to WM_TIMER
chrome_69a00000!base::win::WrappedWindowProc<&base::MessagePumpForUI::WndProcThunk>,
not
remoting_host_plugin!base::win::WrappedWindowProc<&base::MessagePumpForUI::WndProcThunk>:

0:004> kL
ChildEBP RetAddr
03bdfb38 65f7694b
remoting_host_plugin!base::MessagePumpForUI::~MessagePumpForUI+0x2b
03bdfb44 69a913f7 remoting_host_plugin!base::MessagePumpForUI::`scalar
deleting destructor'+0xb
03bdfb60 69a9136c chrome_69a00000!base::MessagePumpForUI::WndProcThunk+0x58
03bdfba8 76f262fa
chrome_69a00000!base::win::WrappedWindowProc<&base::MessagePumpForUI::WndProcThunk>+0x50
03bdfbd4 76f26d3a USER32!InternalCallWinProc+0x23
03bdfc4c 76f277c4 USER32!UserCallWinProcCheckWow+0x109
03bdfcac 76f2788a USER32!DispatchMessageWorker+0x3bc
03bdfcbc 65f76d79 USER32!DispatchMessageW+0xf
03bdfcd0 65f76b43
remoting_host_plugin!base::MessagePumpForUI::ProcessMessageHelper+0x99
03bdfd00 65f764ee
remoting_host_plugin!base::MessagePumpForUI::DoRunLoop+0x43
03bdfd20 65f60b49 remoting_host_plugin!base::MessagePumpWin::Run+0x3e
03bdfd44 65f5ab4b remoting_host_plugin!MessageLoop::Run+0x99
03bdfd4c 65f5abe3 remoting_host_plugin!base::Thread::Run+0xb
03bdfea4 65f5af0b remoting_host_plugin!base::Thread::ThreadMain+0x93
03bdfeb0 7521339a remoting_host_plugin!base::`anonymous
namespace'::ThreadFunc+0x1b
03bdfebc 77909ef2 kernel32!BaseThreadInitThunk+0xe
03bdfefc 77909ec5 ntdll!__RtlUserThreadStart+0x70
03bdff14 00000000 ntdll!_RtlUserThreadStart+0x1b

[chro...@googlecode.com]

Comment #2 on issue 124091 by ser...@chromium.org: Chromoting: the host

plugin calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3
http://code.google.com/p/chromium/issues/detail?id=124091

Do we really need ComThread to be a UI thread? Alternatively we could run
all COM stuff on the plugin main thread.

[chro...@googlecode.com]

Comment #3 on issue 124091 by alexe...@chromium.org: Chromoting: the host

plugin calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3
http://code.google.com/p/chromium/issues/detail?id=124091

> Do we really need ComThread to be a UI thread?

I prefer to keep it a single-threaded apartment (hence a UI thread) until
we really need to make it multi-threaded apartment. It does the same sort
of synchronization that we achieve by posting tasks to message loops
instead on calling functions on random threads.

> Alternatively we could run all COM stuff on the plugin main thread.

It is not going to help it. The problem is that the message loop
implementations in Chrome and the plugin should be completely isolated.
They are currently not, so using timer on any plugin's thread is not safe.
We cannot, however, make plugin times schedule tasks on the chrome's
message loop because we don't access to it anyway.

I'm patching the message loop code to fix this.

[chro...@googlecode.com]

Comment #4 on issue 124091 by bugdro...@chromium.org: Chromoting: the host

plugin calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3

http://code.google.com/p/chromium/issues/detail?id=124091#c4

The following revision refers to this bug:
http://src.chromium.org/viewvc/chrome?view=rev&revision=133134

------------------------------------------------------------------------
r133134 | alex...@chromium.org | Thu Apr 19 19:52:28 PDT 2012

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/message_pump_win.h?r1=133134&r2=133133&pathrev=133134
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/process_util.h?r1=133134&r2=133133&pathrev=133134
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/process_util_win.cc?r1=133134&r2=133133&pathrev=133134
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/message_pump_win.cc?r1=133134&r2=133133&pathrev=133134

Make sure that base::MessagePumpForUI from different modules are isolated
from each other and add protection from shatter attacks by placing |this|
pointer to the used data associated with the message-only window (instead
of blindly trusting the value of WPARAM).

BUG=124091

Review URL: http://codereview.chromium.org/10134001
------------------------------------------------------------------------

[chro...@googlecode.com]

Updates:
Status: Started

Comment #8 on issue 124091 by alexe...@chromium.org: Chromoting: the host

plugin calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3
http://code.google.com/p/chromium/issues/detail?id=124091

The revert r137448 re-introduces this issue in the 1132 branch.

[chro...@googlecode.com]

Comment #9 on issue 124091 by bugdro...@chromium.org: Chromoting: the host

plugin calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3

http://code.google.com/p/chromium/issues/detail?id=124091#c9

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=137542

------------------------------------------------------------------------
r137542 | alex...@chromium.org | Wed May 16 15:29:28 PDT 2012

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/message_pump_win.h?r1=137542&r2=137541&pathrev=137542
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/process_util.h?r1=137542&r2=137541&pathrev=137542
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/process_util_win.cc?r1=137542&r2=137541&pathrev=137542
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/message_pump_win.cc?r1=137542&r2=137541&pathrev=137542

Reverting r133134 - Make sure that base::MessagePumpForUI from different

modules are isolated from each other and add protection from shatter
attacks by placing |this| pointer to the used data associated with the
message-only window (instead of blindly trusting the value of WPARAM).

We suspect that r133134 might be causing issues like 124091, so it needs to
be reworked.

BUG=124091,127933

TBR=alex...@chromium.org

Review URL: https://chromiumcodereview.appspot.com/10384209
------------------------------------------------------------------------

[chro...@googlecode.com]

Comment #10 on issue 124091 by bugdro...@chromium.org: Chromoting: the host

plugin calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3

http://code.google.com/p/chromium/issues/detail?id=124091#c10

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=137547

------------------------------------------------------------------------
r137547 | alex...@chromium.org | Wed May 16 15:46:17 PDT 2012

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/process_util.h?r1=137547&r2=137546&pathrev=137547
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/process_util_win.cc?r1=137547&r2=137546&pathrev=137547

Fix base::GetModuleFromAddress() not to increment the module's reference
count,
to match the behaviour expected by calling code.

The original change was reverted by
https://chromiumcodereview.appspot.com/10384209.

BUG=124091,127933

Review URL: https://chromiumcodereview.appspot.com/10391169
------------------------------------------------------------------------

[chro...@googlecode.com]

Comment #11 on issue 124091 by bugdro...@chromium.org: Chromoting: the host

plugin calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3

http://code.google.com/p/chromium/issues/detail?id=124091#c11

The following revision refers to this bug:

http://src.chromium.org/viewvc/chrome?view=rev&revision=137765

------------------------------------------------------------------------
r137765 | alex...@chromium.org | Thu May 17 16:09:09 PDT 2012

Changed paths:
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/message_pump_win.h?r1=137765&r2=137764&pathrev=137765
M
http://src.chromium.org/viewvc/chrome/trunk/src/base/message_pump_win.cc?r1=137765&r2=137764&pathrev=137765

Making sure that base::MessagePumpForUI from different modules are isolated
from each other.

This is a minimal version of the original fix that was reverted by
https://src.chromium.org/viewvc/chrome?view=rev&revision=137542. This
version does not contain any protection against shatter attacks.

BUG=124091


Review URL: https://chromiumcodereview.appspot.com/10407011
------------------------------------------------------------------------

[chro...@googlecode.com]

Updates:
Labels: -merge-merged-1132 Merge-Requested ReleaseBlock-Beta

Comment #12 on issue 124091 by alexe...@chromium.org: Chromoting: the host

plugin calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3
http://code.google.com/p/chromium/issues/detail?id=124091

Requesting r137547 and r137765 to be merged to 1132 branch. These two CLs
compose a minimal version of the original fix that does not seem to cause
UI message pump hangs. It's been in trunk starting from 21.0.1141.0.

[chro...@googlecode.com]

Comment #13 on issue 124091 by justimag...@gmail.com: Chromoting: the host

plugin calls a pure virtual method at
remoting_host_plugin!MessageLoop::AddToIncomingQueue+0xf3
http://code.google.com/p/chromium/issues/detail?id=124091

hi,
i m new to this project.. i don't know any thing... i can't get these
coding.. can some one assist me please...