author | Randell Jesup <rjesup@jesup.org> |
Sun, 11 Jan 2015 00:28:34 -0500 | |
changeset 223155 | 238f460e7aedc87a4ce3fdff8b9479256d3f3d58 |
parent 223154 | 8f3cc2c90893ab18bcb9eda7961ff3fc08022a99 |
child 223156 | 0aaec62e668467cc9e90e6959c3feccfcf0652bc |
push id | 53838 |
push user | kwierso@gmail.com |
push date | Sun, 11 Jan 2015 20:28:05 +0000 |
treeherder | mozilla-inbound@238f460e7aed [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | bsmedberg |
bugs | 1030372 |
milestone | 37.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
media/mtransport/runnable_utils.h | file | annotate | diff | comparison | revisions | |
netwerk/sctp/datachannel/DataChannel.cpp | file | annotate | diff | comparison | revisions |
--- a/media/mtransport/runnable_utils.h +++ b/media/mtransport/runnable_utils.h @@ -31,17 +31,20 @@ RunOnThreadInternal(nsIEventTarget *thre nsresult rv; rv = thread->IsOnCurrentThread(&on); // If the target thread has already shut down, we don't want to assert. if (rv != NS_ERROR_NOT_INITIALIZED) { MOZ_ASSERT(NS_SUCCEEDED(rv)); } - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + // we're going to destroy the runnable on this thread! + return rv; + } if (!on) { return thread->Dispatch(runnable_ref, flags); } } return runnable_ref->Run(); } template<RunnableResult result>
--- a/netwerk/sctp/datachannel/DataChannel.cpp +++ b/netwerk/sctp/datachannel/DataChannel.cpp @@ -36,16 +36,17 @@ #include "nsIObserver.h" #include "mozilla/Services.h" #include "nsProxyRelease.h" #include "nsThread.h" #include "nsThreadUtils.h" #include "nsAutoPtr.h" #include "nsNetUtil.h" #include "mozilla/StaticPtr.h" +#include "mozilla/unused.h" #ifdef MOZ_PEERCONNECTION #include "mtransport/runnable_utils.h" #endif #define DATACHANNEL_LOG(args) LOG(args) #include "DataChannel.h" #include "DataChannelProtocol.h" @@ -2305,17 +2306,17 @@ DataChannelConnection::SendBinary(DataCh class ReadBlobRunnable : public nsRunnable { public: ReadBlobRunnable(DataChannelConnection* aConnection, uint16_t aStream, nsIInputStream* aBlob) : mConnection(aConnection), mStream(aStream), mBlob(aBlob) - { } + {} NS_IMETHODIMP Run() { // ReadBlob() is responsible to releasing the reference DataChannelConnection *self = mConnection; self->ReadBlob(mConnection.forget(), mStream, mBlob); return NS_OK; } @@ -2344,49 +2345,89 @@ DataChannelConnection::SendBlob(uint16_t } } nsCOMPtr<nsIRunnable> runnable = new ReadBlobRunnable(this, stream, aBlob); mInternalIOThread->Dispatch(runnable, NS_DISPATCH_NORMAL); return 0; } +class DataChannelBlobSendRunnable : public nsRunnable +{ +public: + DataChannelBlobSendRunnable(already_AddRefed<DataChannelConnection>& aConnection, + uint16_t aStream) + : mConnection(aConnection) + , mStream(aStream) {} + + ~DataChannelBlobSendRunnable() + { + if (!NS_IsMainThread() && mConnection) { + MOZ_ASSERT(false); + // explicitly leak the connection if destroyed off mainthread + unused << mConnection.forget().take(); + } + } + + NS_IMETHODIMP Run() + { + ASSERT_WEBRTC(NS_IsMainThread()); + + mConnection->SendBinaryMsg(mStream, mData); + mConnection = nullptr; + return NS_OK; + } + + // explicitly public so we can avoid allocating twice and copying + nsCString mData; + +private: + // Note: we can be destroyed off the target thread, so be careful not to let this + // get Released()ed on the temp thread! + nsRefPtr<DataChannelConnection> mConnection; + uint16_t mStream; +}; + void DataChannelConnection::ReadBlob(already_AddRefed<DataChannelConnection> aThis, uint16_t aStream, nsIInputStream* aBlob) { // NOTE: 'aThis' has been forgotten by the caller to avoid releasing // it off mainthread; if PeerConnectionImpl has released then we want // ~DataChannelConnection() to run on MainThread // XXX to do this safely, we must enqueue these atomically onto the - // output socket. We need a sender thread(s?) to enque data into the + // output socket. We need a sender thread(s?) to enqueue data into the // socket and to avoid main-thread IO that might block. Even on a // background thread, we may not want to block on one stream's data. // I.e. run non-blocking and service multiple channels. // For now as a hack, send as a single blast of queued packets which may // be deferred until buffer space is available. - nsCString temp; uint64_t len; nsCOMPtr<nsIThread> mainThread; NS_GetMainThread(getter_AddRefs(mainThread)); + // Must not let Dispatching it cause the DataChannelConnection to get + // released on the wrong thread. Using WrapRunnable(nsRefPtr<DataChannelConnection>(aThis),... + // will occasionally cause aThis to get released on this thread. Also, an explicit Runnable + // lets us avoid copying the blob data an extra time. + nsRefPtr<DataChannelBlobSendRunnable> runnable = new DataChannelBlobSendRunnable(aThis, + aStream); + // avoid copying the blob data by passing the mData from the runnable if (NS_FAILED(aBlob->Available(&len)) || - NS_FAILED(NS_ReadInputStreamToString(aBlob, temp, len))) { + NS_FAILED(NS_ReadInputStreamToString(aBlob, runnable->mData, len))) { // Bug 966602: Doesn't return an error to the caller via onerror. // We must release DataChannelConnection on MainThread to avoid issues (bug 876167) - NS_ProxyRelease(mainThread, aThis.take()); + // aThis is now owned by the runnable; release it there + NS_ProxyRelease(mainThread, runnable); return; } aBlob->Close(); - RUN_ON_THREAD(mainThread, WrapRunnable(nsRefPtr<DataChannelConnection>(aThis), - &DataChannelConnection::SendBinaryMsg, - aStream, temp), - NS_DISPATCH_NORMAL); + NS_DispatchToMainThread(runnable, NS_DISPATCH_NORMAL); } void DataChannelConnection::GetStreamIds(std::vector<uint16_t>* aStreamList) { ASSERT_WEBRTC(NS_IsMainThread()); for (uint32_t i = 0; i < mStreams.Length(); ++i) { if (mStreams[i]) {