Bug 1280041: more complete fix for issues surrounding mConnection clearing r=drno

This commit is contained in:
Randell Jesup 2016-10-20 01:03:44 -04:00
parent 0d4f94f79a
commit 607b7c3fdf
3 changed files with 44 additions and 17 deletions

View File

@ -116,6 +116,8 @@ NS_IMPL_EVENT_HANDLER(nsDOMDataChannel, error)
NS_IMPL_EVENT_HANDLER(nsDOMDataChannel, close)
NS_IMPL_EVENT_HANDLER(nsDOMDataChannel, message)
// Most of the GetFoo()/SetFoo()s don't need to touch shared resources and
// are safe after Close()
NS_IMETHODIMP
nsDOMDataChannel::GetLabel(nsAString& aLabel)
{
@ -180,7 +182,11 @@ nsDOMDataChannel::ReadyState() const
NS_IMETHODIMP
nsDOMDataChannel::GetReadyState(nsAString& aReadyState)
{
uint16_t readyState = mDataChannel->GetReadyState();
// mState is handled on multiple threads and needs locking
uint16_t readyState = mozilla::DataChannel::CLOSED;
if (!mSentClose) {
readyState = mDataChannel->GetReadyState();
}
// From the WebRTC spec
const char * stateName[] = {
"connecting",
@ -198,7 +204,10 @@ nsDOMDataChannel::GetReadyState(nsAString& aReadyState)
uint32_t
nsDOMDataChannel::BufferedAmount() const
{
return mDataChannel->GetBufferedAmount();
if (!mSentClose) {
return mDataChannel->GetBufferedAmount();
}
return 0;
}
uint32_t
@ -328,7 +337,10 @@ nsDOMDataChannel::Send(nsIInputStream* aMsgStream,
ErrorResult& aRv)
{
MOZ_ASSERT(NS_IsMainThread());
uint16_t state = mDataChannel->GetReadyState();
uint16_t state = mozilla::DataChannel::CLOSED;
if (!mSentClose) {
state = mDataChannel->GetReadyState();
}
// In reality, the DataChannel protocol allows this, but we want it to
// look like WebSockets
@ -465,6 +477,8 @@ nsDOMDataChannel::OnChannelClosed(nsISupports* aContext)
// so we don't have to worry if we're notified from different paths in
// the underlying code
if (!mSentClose) {
// Ok, we're done with it.
mDataChannel->ReleaseConnection();
LOG(("%p(%p): %s - Dispatching\n",this,(void*)mDataChannel,__FUNCTION__));
rv = OnSimpleEvent(aContext, NS_LITERAL_STRING("close"));
@ -496,7 +510,9 @@ nsDOMDataChannel::NotBuffered(nsISupports* aContext)
void
nsDOMDataChannel::AppReady()
{
mDataChannel->AppReady();
if (!mSentClose) { // may not be possible, simpler to just test anyways
mDataChannel->AppReady();
}
}
//-----------------------------------------------------------------------------

View File

@ -1748,15 +1748,12 @@ DataChannelConnection::HandleStreamResetEvent(const struct sctp_stream_reset_eve
// Mark the stream for reset (the reset is sent below)
ResetOutgoingStream(channel->mStream);
}
NS_DispatchToMainThread(do_AddRef(new DataChannelOnMessageAvailable(
DataChannelOnMessageAvailable::ON_CHANNEL_CLOSED, this,
channel)));
mStreams[channel->mStream] = nullptr;
LOG(("Disconnected DataChannel %p from connection %p",
(void *) channel.get(), (void *) channel->mConnection.get()));
channel->DestroyLocked();
// At this point when we leave here, the object is a zombie held alive only by the DOM object
// This sends ON_CHANNEL_CLOSED to mainthread
channel->StreamClosedLocked();
} else {
LOG(("Can't find incoming channel %d",i));
}
@ -2492,7 +2489,7 @@ DataChannelConnection::CloseInt(DataChannel *aChannel)
aChannel->mState = CLOSING;
if (mState == CLOSED) {
// we're not going to hang around waiting
channel->DestroyLocked();
channel->StreamClosedLocked();
}
// At this point when we leave here, the object is a zombie held alive only by the DOM object
}
@ -2545,14 +2542,16 @@ DataChannel::~DataChannel()
void
DataChannel::Close()
{
ENSURE_DATACONNECTION;
RefPtr<DataChannelConnection> connection(mConnection);
connection->Close(this);
if (mConnection) {
// ensure we don't get deleted
RefPtr<DataChannelConnection> connection(mConnection);
connection->Close(this);
}
}
// Used when disconnecting from the DataChannelConnection
void
DataChannel::DestroyLocked()
DataChannel::StreamClosedLocked()
{
mConnection->mLock.AssertCurrentThreadOwns();
ENSURE_DATACONNECTION;
@ -2565,6 +2564,13 @@ DataChannel::DestroyLocked()
NS_DispatchToMainThread(do_AddRef(new DataChannelOnMessageAvailable(
DataChannelOnMessageAvailable::ON_CHANNEL_CLOSED,
mConnection, this)));
// We leave mConnection live until the DOM releases us, to avoid races
}
void
DataChannel::ReleaseConnection()
{
ASSERT_WEBRTC(NS_IsMainThread());
mConnection = nullptr;
}

View File

@ -285,10 +285,10 @@ private:
};
#define ENSURE_DATACONNECTION \
do { if (!mConnection) { DATACHANNEL_LOG(("%s: %p no connection!",__FUNCTION__, this)); return; } } while (0)
do { MOZ_ASSERT(mConnection); if (!mConnection) { return; } } while (0)
#define ENSURE_DATACONNECTION_RET(x) \
do { if (!mConnection) { DATACHANNEL_LOG(("%s: %p no connection!",__FUNCTION__, this)); return (x); } } while (0)
do { MOZ_ASSERT(mConnection); if (!mConnection) { return (x); } } while (0)
class DataChannel {
public:
@ -334,7 +334,12 @@ public:
NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DataChannel)
// when we disconnect from the connection after stream RESET
void DestroyLocked();
void StreamClosedLocked();
// Complete dropping of the link between DataChannel and the connection.
// After this, except for a few methods below listed to be safe, you can't
// call into DataChannel.
void ReleaseConnection();
// Close this DataChannel. Can be called multiple times. MUST be called
// before destroying the DataChannel (state must be CLOSED or CLOSING).