New issue
Advanced search Search tips

Issue 634218 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Aug 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

[Windows][Host] Native Messaging Hosts hang when reporting an error

Project Member Reported by joedow@chromium.org, Aug 4 2016

Issue description

The Me2Me and It2Me native messaging hosts do not exit cleanly when they close their side of the NMH communication channel.

I've tracked this down to a hang in the native message reader, that class uses a separate thread to perform blocking IO, however since the read operation is blocking, it also prevents the thread from being cleaned up properly when the containing object is destroyed (which leads to the hang).
 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 10 2016

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

commit 0edf3b33e71c5cd32e6fe02497ee4f17152a9772
Author: joedow <joedow@chromium.org>
Date: Wed Aug 10 20:09:02 2016

Fixing a hang in the native messaging hosts on Windows

The It2Me and Me2Me native messaging host processes would hang after reporting
an error and attempting to terminate the process.  This appeared to be caused
by the NativeMessageReader component which was blocked on a read operation on
one thread while another thread was waiting for it to exit.  The fix is to use
a Windows API to cancel synchronous IO on the blocked thread and then
destroying the inner message reader class.

Also, we should treat an elevate request cancellation the same as a failure to
launch the elevated process.  The binary cannot accomplish its task either way
so there isn't any benefit to treating them differently, it's better to allow
the caller to decide what to do in that case (restart the NMH or send another
message to it).

BUG= 634218 

Review-Url: https://codereview.chromium.org/2207153004
Cr-Commit-Position: refs/heads/master@{#411125}

[modify] https://crrev.com/0edf3b33e71c5cd32e6fe02497ee4f17152a9772/remoting/host/native_messaging/native_messaging_reader.cc
[modify] https://crrev.com/0edf3b33e71c5cd32e6fe02497ee4f17152a9772/remoting/host/native_messaging/native_messaging_reader_unittest.cc
[modify] https://crrev.com/0edf3b33e71c5cd32e6fe02497ee4f17152a9772/remoting/host/setup/me2me_native_messaging_host.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 10 2016

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

commit 0edf3b33e71c5cd32e6fe02497ee4f17152a9772
Author: joedow <joedow@chromium.org>
Date: Wed Aug 10 20:09:02 2016

Fixing a hang in the native messaging hosts on Windows

The It2Me and Me2Me native messaging host processes would hang after reporting
an error and attempting to terminate the process.  This appeared to be caused
by the NativeMessageReader component which was blocked on a read operation on
one thread while another thread was waiting for it to exit.  The fix is to use
a Windows API to cancel synchronous IO on the blocked thread and then
destroying the inner message reader class.

Also, we should treat an elevate request cancellation the same as a failure to
launch the elevated process.  The binary cannot accomplish its task either way
so there isn't any benefit to treating them differently, it's better to allow
the caller to decide what to do in that case (restart the NMH or send another
message to it).

BUG= 634218 

Review-Url: https://codereview.chromium.org/2207153004
Cr-Commit-Position: refs/heads/master@{#411125}

[modify] https://crrev.com/0edf3b33e71c5cd32e6fe02497ee4f17152a9772/remoting/host/native_messaging/native_messaging_reader.cc
[modify] https://crrev.com/0edf3b33e71c5cd32e6fe02497ee4f17152a9772/remoting/host/native_messaging/native_messaging_reader_unittest.cc
[modify] https://crrev.com/0edf3b33e71c5cd32e6fe02497ee4f17152a9772/remoting/host/setup/me2me_native_messaging_host.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 10 2016

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

commit 0edf3b33e71c5cd32e6fe02497ee4f17152a9772
Author: joedow <joedow@chromium.org>
Date: Wed Aug 10 20:09:02 2016

Fixing a hang in the native messaging hosts on Windows

The It2Me and Me2Me native messaging host processes would hang after reporting
an error and attempting to terminate the process.  This appeared to be caused
by the NativeMessageReader component which was blocked on a read operation on
one thread while another thread was waiting for it to exit.  The fix is to use
a Windows API to cancel synchronous IO on the blocked thread and then
destroying the inner message reader class.

Also, we should treat an elevate request cancellation the same as a failure to
launch the elevated process.  The binary cannot accomplish its task either way
so there isn't any benefit to treating them differently, it's better to allow
the caller to decide what to do in that case (restart the NMH or send another
message to it).

BUG= 634218 

Review-Url: https://codereview.chromium.org/2207153004
Cr-Commit-Position: refs/heads/master@{#411125}

[modify] https://crrev.com/0edf3b33e71c5cd32e6fe02497ee4f17152a9772/remoting/host/native_messaging/native_messaging_reader.cc
[modify] https://crrev.com/0edf3b33e71c5cd32e6fe02497ee4f17152a9772/remoting/host/native_messaging/native_messaging_reader_unittest.cc
[modify] https://crrev.com/0edf3b33e71c5cd32e6fe02497ee4f17152a9772/remoting/host/setup/me2me_native_messaging_host.cc

Comment 5 by joedow@chromium.org, Aug 10 2016

Status: Fixed (was: Started)

Comment 6 by joedow@chromium.org, Aug 10 2016

Status: Verified (was: Fixed)

Sign in to add a comment