New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 9 users
Status: Available
Owner: ----
Cc:
Components:
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment
Closing peer connection during native PeerConnection callbacks leads to crash
Reported by andrewgr...@gmail.com, Aug 20 2014 Back to list
What steps will reproduce the problem?

1. Start outgoing call and wait untill local offer will be successfully created and set as local description
2. Drop call after ICE gatering state become GATHERING


What is the expected result?

call finishes correctly


What do you see instead?

application crasshes with message
I/DEBUG   ( 2555): signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 00000000

What version of the product are you using? On what operating system?

latest revision of webrtc source
OS: Android 4.4.3/4.4.4 
Devices: Samsung Galaxy S4, Nexus 5

Please provide any additional information below.

by modifying default demo with next code: 

@Override
public void onIceGatheringChange(PeerConnection.IceGatheringState iceGatheringState) {       if(PeerConnection.IceGatheringState.GATHERING.equals(iceGatheringState)) {
                closePeerConnection();
    }
}

I managed to reproduce this failure in 100% cases.
 
webRTC_failure_trace.txt
6.4 KB View Download
Project Member Comment 1 by juberti@webrtc.org, Aug 21 2014
Cc: glaznev@webrtc.org
Owner: jiayl@webrtc.org
Does this occur on desktop, or only Android?
I haven't tried it on desktop as I have no customizable desktop application for this.
Project Member Comment 3 by jiayl@webrtc.org, Aug 21 2014
Cc: juberti@webrtc.org
Status: Assigned
I can reproduce it on Linux by modifying the PeerConnectionEndToEnd unittest. The call stack is like:

#0  0x00007ffff4f1e4f5 in __GI_raise (sig=<optimized out>)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x00007ffff4f21c5b in __GI_abort () at abort.c:91
#2  0x00007ffff573d5ad in __gnu_debug::_Error_formatter::_M_error (
    this=0x7fffffffce98) at ../../../../src/libstdc++-v3/src/debug.cc:611
Python Exception <type 'exceptions.ValueError'> Cannot find type std::__cxx1998::_List_const_iterator<sigslot::_connection_base1<cricket::Transport*, sigslot::single_threaded>*>::_Node: 
#3  0x0000000000778498 in __gnu_debug::_Safe_iterator<std::__cxx1998::_List_const_iterator<sigslot::_connection_base1<cricket::Transport*, sigslot::single_threaded>*>, std::__debug::list<sigslot::_connection_base1<cricket::Transport*, sigslot::single_threaded>*, std::allocator<sigslot::_connection_base1<cricket::Transport*, sigslot::single_threaded>*> > >::operator= (this=0x7fffffffd1a8, __x=)
    at /usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/debug/safe_iterator.h:174
#4  0x0000000000771cba in sigslot::signal1<cricket::Transport*, sigslot::single_threaded>::operator() (this=0x1c52a68, a1=0x1c528f0)
    at ../../webrtc/base/sigslot.h:2348
#5  0x000000000076f17f in cricket::Transport::OnChannelRequestSignaling_s (
    this=0x1c528f0, component=1) at ../../talk/p2p/base/transport.cc:591
#6  0x000000000076ffb2 in cricket::Transport::OnMessage (this=0x1c528f0, 
    msg=0x7fffffffd520) at ../../talk/p2p/base/transport.cc:916
#7  0x00000000009fecb2 in rtc::MessageQueue::Dispatch (this=0x159b740, 
    pmsg=0x7fffffffd520) at ../../webrtc/base/messagequeue.cc:381
#8  0x0000000000a4e7d6 in rtc::Thread::ProcessMessages (this=0x159b740, 
    cmsLoop=1) at ../../webrtc/base/thread.cc:518
#9  0x00000000005ed443 in PeerConnectionTestWrapper::WaitForConnection (
    this=0x15848b0)

This is because the Transport is deleted while it was emitting a signal (SignalConnecting).
The crash only happens in native apps, because Chrome always fires the JS callback async.

This could affect other callbacks from the Transport too, e.g. IceConnectionState, onIceCandidate.


Justin,

should we change webrtc::PeerConnection to fire all these callbacks async?

Yes, all the callbacks should be done async to prevent us from exploding when someone calls Close() in response to a callback.

Shall we adjust the bug title to reflect this?

Project Member Comment 5 by jiayl@webrtc.org, Sep 2 2014
Labels: Mstone-39
Summary: Closing peer connection during PeerConnection callbacks leads to crash (was: Closing peer connection during ICE gartering leads to failure)
Project Member Comment 6 by jiayl@webrtc.org, Sep 10 2014
It's unclear how to make all callbacks async, as some callbacks pass a pointer of an object either temporary or not guaranteed to live after the call, e.g. PeerConnectionObserver::OnAddStream.
Project Member Comment 7 by jiayl@webrtc.org, Sep 10 2014
Summary: Closing peer connection during native PeerConnection callbacks leads to crash (was: Closing peer connection during PeerConnection callbacks leads to crash)
Project Member Comment 8 by jiayl@webrtc.org, Oct 7 2014
Labels: -Mstone-39 Mstone-40
Comment 9 by vrk@webrtc.org, Oct 14 2014
Labels: Area-PeerConnection
Comment 10 by vrk@webrtc.org, Oct 16 2014
Labels: EngTriaged
Project Member Comment 11 by jiayl@webrtc.org, Oct 31 2014
Labels: -Mstone-40
Removing the milestone as it does not affect Chrome, but only native apps.
Project Member Comment 12 by tnakamura@webrtc.org, Nov 4 2015
This bug hasn't been modified for more than a year. Is this still a valid open issue?
Project Member Comment 13 by deadbeef@webrtc.org, Nov 12 2016
Cc: deadbeef@webrtc.org
Owner: ----
Status: Available
This is definitely a valid issue. Someone comes to me with this at least once a month, and it always takes a while to figure out because the crash occurs in an odd place like a sigslot::signal being fired.

So, I agree with Justin's recommendation in comment #4. I don't think the issue in comment #6 prevents us from doing this, because the objects we pass in these events are refcounted and we have rtc::Bind to help us with refcounts in async operations.
Project Member Comment 14 by bugdroid1@chromium.org, Mar 25 2017
Project Member Comment 15 by deadbeef@webrtc.org, Aug 31
Note that this issue came up again, for an application trying to dispose of the PeerConnection from a DataChannel onStateChange callback: https://bugs.chromium.org/p/webrtc/issues/detail?id=6924
Project Member Comment 16 by bugdroid1@chromium.org, Sep 12
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/43697f6da56c0c6eca8967a4fef57beb589d8990

commit 43697f6da56c0c6eca8967a4fef57beb589d8990
Author: deadbeef <deadbeef@webrtc.org>
Date: Tue Sep 12 17:52:14 2017

Add javadoc comment for PeerConnection.dispose.

Specifically calling out issue 3721 ("dispose can't be called from a
callback"), which developers frequently run into.

BUG=webrtc:3721
NOTRY=True

Review-Url: https://codereview.webrtc.org/3013573002
Cr-Commit-Position: refs/heads/master@{#19804}

[modify] https://crrev.com/43697f6da56c0c6eca8967a4fef57beb589d8990/webrtc/sdk/android/api/org/webrtc/PeerConnection.java

Sign in to add a comment