New issue
Advanced search Search tips

Issue 793295 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 775415



Sign in to add a comment

Unregister PeerConnectionTracker on PC close()

Project Member Reported by hbos@chromium.org, Dec 8 2017

Issue description

The PeerConnectionTracker should be unregistered when the PC is no longer active. This currently only happens in the pc handler's destructor:
https://cs.chromium.org/chromium/src/content/renderer/media/rtc_peer_connection_handler.cc?q=rtc_peer_connection_handler.cc&sq=package:chromium&dr&l=1294

The handler is destroyed if the RTCPeerConnection's context is destroyed or it is garbage collected, this is not something we can rely on happening.

It makes more sense to unregister when the PC is closed.

This was attempted here: https://chromium-review.googlesource.com/c/chromium/src/+/754838
Which for unknown reason caused OnSuspend-related crashes and was reverted:  https://crbug.com/783010 

It looks like something is referencing the tracker after it is destroyed and the host is still dispatching OnSuspend (e.g. laptop lid closes?).

With the WeakPtr support of the tracker I thought the IPC message would not be processed so not sure what's going on. Need to investigate.
 

Comment 1 by hbos@chromium.org, Dec 8 2017

Components: Blink>WebRTC>PeerConnection
Should I mark this as blocking one of your bugs, eladalon?

Comment 2 by hbos@chromium.org, Dec 8 2017

Description: Show this description
It will block the completion of https://bugs.chromium.org/p/chromium/issues/detail?id=775415, though progress is still possible for now.

Comment 4 by hbos@chromium.org, Dec 8 2017

Blocking: 775415

Comment 5 by hbos@chromium.org, Dec 8 2017

I can't make sense of the stack trace and code, the reported crash is invalid memory close to 0x0, it looks like the thing that is sending the IPC message to the Tracker is also the owner of it... I don't see what could be null.

And the Host is on the other process, so that's not it.

I think we would have to find a repro case to debug it further.

Sign in to add a comment