New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 718871 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Crash in Native Message Host binaries when shutting down

Project Member Reported by joedow@chromium.org, May 5 2017

Issue description

I have found an AV in our NMH binaries which can occur if the last message we send to the host results in the pipeline being torn down.

This doesn't affect functionality as it occurs during clean-up but we should prevent any crash dialogs or telemetry from being sent.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 5 2017

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

commit e0ca3279b3b7ba5053a56f03fd57f7feb03d93ba
Author: joedow <joedow@chromium.org>
Date: Fri May 05 14:37:02 2017

Fixing a crash in our native messaging host binaries

This CL fixes a crash which can occur if the native messaging pipeline is
torn down immediately after a message is sent.  The LogMessageHandler class
will run a messaging delegate and then attempt to update a member variable.
If the instance is destroyed, then an AV will occur.

The fix is to check if the instance is still alive using a WeakPtr skip the
member variable assignment if it has been torn down.

BUG= 718871 

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

[modify] https://crrev.com/e0ca3279b3b7ba5053a56f03fd57f7feb03d93ba/remoting/host/native_messaging/log_message_handler.cc

Labels: Merge-Request-59
Requesting merge for crash fix which does not affect the Chrome Browser (Chrome Remote Desktop only).
Project Member

Comment 3 by sheriffbot@chromium.org, May 6 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 4 by bugdroid1@chromium.org, May 6 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/13d77cbc3998a37f2a8e895831af67e7c0bf3752

commit 13d77cbc3998a37f2a8e895831af67e7c0bf3752
Author: Joe Downing <joedow@chromium.org>
Date: Sat May 06 23:12:37 2017

Fixing a crash in our native messaging host binaries

This CL fixes a crash which can occur if the native messaging pipeline is
torn down immediately after a message is sent.  The LogMessageHandler class
will run a messaging delegate and then attempt to update a member variable.
If the instance is destroyed, then an AV will occur.

The fix is to check if the instance is still alive using a WeakPtr skip the
member variable assignment if it has been torn down.

BUG= 718871 

Review-Url: https://codereview.chromium.org/2857983008
Cr-Commit-Position: refs/heads/master@{#469646}
(cherry picked from commit e0ca3279b3b7ba5053a56f03fd57f7feb03d93ba)

Review-Url: https://codereview.chromium.org/2867623002 .
Cr-Commit-Position: refs/branch-heads/3071@{#434}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/13d77cbc3998a37f2a8e895831af67e7c0bf3752/remoting/host/native_messaging/log_message_handler.cc

Labels: M-59
Owner: ajnolley@chromium.org
Status: Fixed (was: Assigned)
Fixed in M60 and merged back to M59.
Status: Verified (was: Fixed)
Verified Fixed in Host version 59.0.3071.47. 

Sign in to add a comment