New issue
Advanced search Search tips

Issue 840492 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 3
Type: Bug



Sign in to add a comment

[CRD iOS] Crash when session disconnects

Project Member Reported by yuweih@chromium.org, May 7 2018

Issue description

We are still seeing quite a few crashes happened when the session disconnects. It looks like the weak pointers are not properly invalidated.
 
Screen Shot 2018-05-07 at 11.46.06 AM.png
107 KB View Download
Summary: [CRD iOS] Crash when session disconnects (was: [CRD iOS] Crash at Core::EnableVideoChannel)
Description: Show this description
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 14

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

commit 0d0f9b5a23ff25b5233195784189fc6a75b66706
Author: Yuwei Huang <yuweih@chromium.org>
Date: Tue Aug 14 00:48:11 2018

[CRD iOS] Fix crashes when session disconnects

We are still seeing quite a few crashes happen when session disconnects
remotely. Here are two issues that I have found:

* It turns out calling WeakPtrFactory::InvalidateWeakPtrs() only
  invalidate WeakPtrs that are currently created, while calling
  WeakPtrFactory::GetWeakPtr() again will create new valid WeakPtrs.
* ChromotingClient owns IceTransport, which runs a timer to periodically
  send pending transport messages. The timer should be stopped
  immediately after the session is disconnected, so we need to
  immediately destroy the ChromotingClient instance in this case.

Bug:  840492 
Change-Id: I6625af62001b0cdfb5745dcc863ed59617e440cf
Reviewed-on: https://chromium-review.googlesource.com/1170174
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582771}
[modify] https://crrev.com/0d0f9b5a23ff25b5233195784189fc6a75b66706/remoting/client/chromoting_session.cc
[modify] https://crrev.com/0d0f9b5a23ff25b5233195784189fc6a75b66706/remoting/client/chromoting_session.h
[modify] https://crrev.com/0d0f9b5a23ff25b5233195784189fc6a75b66706/remoting/client/jni/jni_client.cc
[modify] https://crrev.com/0d0f9b5a23ff25b5233195784189fc6a75b66706/remoting/ios/session/remoting_client.mm

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 15

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

commit 27d0e8428cc19f6901063217e2e14f0846af8b77
Author: Yuwei Huang <yuweih@chromium.org>
Date: Wed Aug 15 00:04:54 2018

[CRD iOS] Cleanup ChromotingSession and ClientTelemetryLogger

This is a follow up cleanup CL for ChromotingSession crash fixing:

* Make ChromotingSession::Core own the telemetry logger and responsible
  for providing feedback data.
* Make the logger and runtime their own parameter when constructing the
  core, since they don't share the same lifetime as |session_context_|.
* Add DCHECK for |audio_player|.
* Remove WeakPtr from ClientTelemetryLogger.

Bug:  840492 
Change-Id: I054c9a3317cbf17f7a45e6eae61c0f34018f9ca1
Reviewed-on: https://chromium-review.googlesource.com/1173738
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583092}
[modify] https://crrev.com/27d0e8428cc19f6901063217e2e14f0846af8b77/remoting/client/chromoting_session.cc
[modify] https://crrev.com/27d0e8428cc19f6901063217e2e14f0846af8b77/remoting/client/chromoting_session.h
[modify] https://crrev.com/27d0e8428cc19f6901063217e2e14f0846af8b77/remoting/client/client_telemetry_logger.cc
[modify] https://crrev.com/27d0e8428cc19f6901063217e2e14f0846af8b77/remoting/client/client_telemetry_logger.h

Status: Fixed (was: Assigned)
Labels: M-70

Sign in to add a comment