Fix for OnSctpInboundPacket being called after transport destruction.

OnSctpInboundPacket is called not only for incoming packets, but for
notifications, which can be delivered on the usrsctp timer thread.

I suspect that these notifications can be delivered after we attempt to
close the socket, because if we attempt to close it while the timer
thread holds a reference, it isn't actually destroyed until the timer
thread finishes its operation.

Bug: chromium:1127774
Change-Id: Id6a883b14796e8f5bf1c2990f3d9d389d72c8a46
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/184240
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Taylor <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32204}
This commit is contained in:
Taylor Brandstetter 2020-09-24 15:32:25 -07:00 committed by Commit Bot
parent 16db7fff49
commit 9def3ff4a3

View File

@ -393,11 +393,16 @@ class SctpTransport::UsrSctpWrapper {
struct sctp_rcvinfo rcv,
int flags,
void* ulp_info) {
SctpTransport* transport = GetTransportFromSocket(sock);
if (!transport) {
RTC_LOG(LS_ERROR)
<< "OnSctpInboundPacket: Failed to get transport for socket " << sock
<< "; possibly was already destroyed.";
return 0;
}
// Sanity check that both methods of getting the SctpTransport pointer
// yield the same result.
RTC_CHECK_EQ(static_cast<SctpTransport*>(ulp_info),
GetTransportFromSocket(sock));
SctpTransport* transport = static_cast<SctpTransport*>(ulp_info);
RTC_CHECK_EQ(transport, static_cast<SctpTransport*>(ulp_info));
int result =
transport->OnDataOrNotificationFromSctp(data, length, rcv, flags);
free(data);
@ -440,7 +445,7 @@ class SctpTransport::UsrSctpWrapper {
if (!transport) {
RTC_LOG(LS_ERROR)
<< "SendThresholdCallback: Failed to get transport for socket "
<< sock;
<< sock << "; possibly was already destroyed.";
return 0;
}
transport->OnSendThresholdCallback();
@ -450,14 +455,19 @@ class SctpTransport::UsrSctpWrapper {
static int SendThresholdCallback(struct socket* sock,
uint32_t sb_free,
void* ulp_info) {
// Sanity check that both methods of getting the SctpTransport pointer
// yield the same result.
RTC_CHECK_EQ(static_cast<SctpTransport*>(ulp_info),
GetTransportFromSocket(sock));
// Fired on our I/O thread. SctpTransport::OnPacketReceived() gets
// a packet containing acknowledgments, which goes into usrsctp_conninput,
// and then back here.
SctpTransport* transport = static_cast<SctpTransport*>(ulp_info);
SctpTransport* transport = GetTransportFromSocket(sock);
if (!transport) {
RTC_LOG(LS_ERROR)
<< "SendThresholdCallback: Failed to get transport for socket "
<< sock << "; possibly was already destroyed.";
return 0;
}
// Sanity check that both methods of getting the SctpTransport pointer
// yield the same result.
RTC_CHECK_EQ(transport, static_cast<SctpTransport*>(ulp_info));
transport->OnSendThresholdCallback();
return 0;
}
@ -1122,8 +1132,8 @@ int SctpTransport::OnDataOrNotificationFromSctp(void* data,
// If data is NULL, the SCTP association has been closed.
if (!data) {
RTC_LOG(LS_INFO) << debug_name_
<< "->OnSctpInboundPacket(...): "
"No data, closing.";
<< "->OnDataOrNotificationFromSctp(...): "
"No data; association closed.";
return kSctpSuccessReturn;
}
@ -1132,9 +1142,10 @@ int SctpTransport::OnDataOrNotificationFromSctp(void* data,
// be handled early and entirely separate from the reassembly
// process.
if (flags & MSG_NOTIFICATION) {
RTC_LOG(LS_VERBOSE) << debug_name_
<< "->OnSctpInboundPacket(...): SCTP notification"
<< " length=" << length;
RTC_LOG(LS_VERBOSE)
<< debug_name_
<< "->OnDataOrNotificationFromSctp(...): SCTP notification"
<< " length=" << length;
// Copy and dispatch asynchronously
rtc::CopyOnWriteBuffer notification(reinterpret_cast<uint8_t*>(data),
@ -1148,7 +1159,7 @@ int SctpTransport::OnDataOrNotificationFromSctp(void* data,
// Log data chunk
const uint32_t ppid = rtc::NetworkToHost32(rcv.rcv_ppid);
RTC_LOG(LS_VERBOSE) << debug_name_
<< "->OnSctpInboundPacket(...): SCTP data chunk"
<< "->OnDataOrNotificationFromSctp(...): SCTP data chunk"
<< " length=" << length << ", sid=" << rcv.rcv_sid
<< ", ppid=" << ppid << ", ssn=" << rcv.rcv_ssn
<< ", cum-tsn=" << rcv.rcv_cumtsn