Free SCTP data channels asynchronously in PeerConnection.

Free SCTP data channels asynchronously in PeerConnection.

This is needed so that the data channel isn't deleted while one of its
own methods is on the call stack.

BUG=565048
Committed: https://crrev.com/386869247f28e72a00307a1b5c92465eea343ad2
Cr-Commit-Position: refs/heads/master@{#10923}

[Taylor Brandstetter, 2015-12-03 19:19:27]

deadbeef@webrtc.org changed reviewers:
+ pthatcher@webrtc.org

[pthatcher1, 2015-12-03 19:45:15]

Is it possible to write a unit test for this?

https://codereview.webrtc.org/1492383002/diff/1/talk/app/webrtc/peerconnectio...
File talk/app/webrtc/peerconnection.cc (right):

https://codereview.webrtc.org/1492383002/diff/1/talk/app/webrtc/peerconnectio...
talk/app/webrtc/peerconnection.cc:1321: delete param;
Does "delete msg->pdata" not work without the cast?

https://codereview.webrtc.org/1492383002/diff/1/talk/app/webrtc/peerconnectio...
talk/app/webrtc/peerconnection.cc:1914: this, MSG_FREE_DATACHANNEL,
If you don't need the type cast, you could just call this MSG_DELETE.

[Taylor Brandstetter, 2015-12-04 19:27:45]

I'm not sure how to go about writing a unit test. Would it be a test that only
runs under ASan? Is there any precedent for that?

https://codereview.webrtc.org/1492383002/diff/1/talk/app/webrtc/peerconnectio...
File talk/app/webrtc/peerconnection.cc (right):

https://codereview.webrtc.org/1492383002/diff/1/talk/app/webrtc/peerconnectio...
talk/app/webrtc/peerconnection.cc:1914: this, MSG_FREE_DATACHANNEL,
On 2015/12/03 19:45:15, pthatcher1 wrote:
> If you don't need the type cast, you could just call this MSG_DELETE. 

Good point. Done.

[Taylor Brandstetter, 2015-12-04 21:03:56]

Ok, added unit test. I verified that this catches the issue if it's introduced
again.

[pthatcher1, 2015-12-05 01:30:57]

lgtm

[Taylor Brandstetter, 2015-12-07 19:31:42]

The CQ bit was checked by deadbeef@webrtc.org

[commit-bot: I haz the power, 2015-12-07 19:31:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1492383002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1492383002/40001

[commit-bot: I haz the power, 2015-12-07 21:32:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-07 21:32:37]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[Taylor Brandstetter, 2015-12-07 23:31:21]

The CQ bit was checked by deadbeef@webrtc.org

[commit-bot: I haz the power, 2015-12-07 23:31:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1492383002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1492383002/40001

[commit-bot: I haz the power, 2015-12-07 23:32:26]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-12-07 23:32:39]

Description was changed from

==========
Free SCTP data channels asynchronously in PeerConnection.

This is needed so that the data channel isn't deleted while one of its
own methods is on the call stack.

BUG=565048
==========

to

==========
Free SCTP data channels asynchronously in PeerConnection.

This is needed so that the data channel isn't deleted while one of its
own methods is on the call stack.

BUG=565048

Committed: https://crrev.com/386869247f28e72a00307a1b5c92465eea343ad2
Cr-Commit-Position: refs/heads/master@{#10923}
==========

[commit-bot: I haz the power, 2015-12-07 23:32:39]

Patchset 3 (id:??) landed as
https://crrev.com/386869247f28e72a00307a1b5c92465eea343ad2
Cr-Commit-Position: refs/heads/master@{#10923}

[Taylor Brandstetter, 2015-12-10 19:17:23]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.webrtc.org/1513143003/ by deadbeef@webrtc.org.

The reason for reverting is: Breaks WebrtcTransportTest.DataStream, due to
different rtc::Thread implementation on Chromium..