Bug 634534 - Firefox 4 (and probably earlier versions) crash when they make a byte-range request and then call NPN_DestroyStream while both the "main" request and the byte-range request are active. This affects Silverlight on mac, and almost certainly Acrobat on Windows.

Instead of tracking a single request in nsPluginStreamListenerPeer::mRequest, track all the currently pending requests with nsPluginStreamListenerPeer::mRequests, and make sure that they are all cancelled when the stream is destroyed. r=josh sr=bz a=blocker
This commit is contained in:
Benjamin Smedberg 2011-02-23 10:47:25 -05:00
parent 1481e5c51f
commit b70550d556
5 changed files with 177 additions and 43 deletions

View File

@ -361,7 +361,7 @@ nsNPAPIPluginStreamListener::OnStartBinding(nsIPluginStreamInfo* pluginInfo)
return NS_OK;
}
nsresult
void
nsNPAPIPluginStreamListener::SuspendRequest()
{
NS_ASSERTION(!mIsSuspended,
@ -369,19 +369,18 @@ nsNPAPIPluginStreamListener::SuspendRequest()
nsCOMPtr<nsINPAPIPluginStreamInfo> pluginInfoNPAPI =
do_QueryInterface(mStreamInfo);
nsIRequest *request;
if (!pluginInfoNPAPI || !(request = pluginInfoNPAPI->GetRequest())) {
NS_ERROR("Trying to suspend a non-suspendable stream!");
return NS_ERROR_FAILURE;
if (!pluginInfoNPAPI) {
return;
}
nsresult rv = StartDataPump();
NS_ENSURE_SUCCESS(rv, rv);
if (NS_FAILED(rv))
return;
mIsSuspended = PR_TRUE;
return request->Suspend();
pluginInfoNPAPI->SuspendRequests();
}
void
@ -390,11 +389,7 @@ nsNPAPIPluginStreamListener::ResumeRequest()
nsCOMPtr<nsINPAPIPluginStreamInfo> pluginInfoNPAPI =
do_QueryInterface(mStreamInfo);
nsIRequest *request = pluginInfoNPAPI->GetRequest();
// request can be null if the network stream is done.
if (request)
request->Resume();
pluginInfoNPAPI->ResumeRequests();
mIsSuspended = PR_FALSE;
}
@ -609,7 +604,7 @@ nsNPAPIPluginStreamListener::OnDataAvailable(nsIPluginStreamInfo* pluginInfo,
if (numtowrite <= 0 ||
(!mIsPluginInitJSStream && PluginInitJSLoadInProgress())) {
if (!mIsSuspended) {
rv = SuspendRequest();
SuspendRequest();
}
// Break out of the inner loop, but keep going through the
@ -676,7 +671,7 @@ nsNPAPIPluginStreamListener::OnDataAvailable(nsIPluginStreamInfo* pluginInfo,
// resume the request.
if (mIsSuspended || ++zeroBytesWriteCount == 3) {
if (!mIsSuspended) {
rv = SuspendRequest();
SuspendRequest();
}
// Break out of the for loop, but keep going through the
@ -758,9 +753,8 @@ nsNPAPIPluginStreamListener::OnStopBinding(nsIPluginStreamInfo* pluginInfo,
nsCOMPtr<nsINPAPIPluginStreamInfo> pluginInfoNPAPI =
do_QueryInterface(mStreamInfo);
nsIRequest *request;
if (pluginInfoNPAPI && (request = pluginInfoNPAPI->GetRequest())) {
request->Cancel(status);
if (pluginInfoNPAPI) {
pluginInfoNPAPI->CancelRequests(status);
}
}

View File

@ -45,7 +45,7 @@
#include "nsIRequest.h"
#include "nsITimer.h"
#include "nsAutoPtr.h"
#include "nsCOMPtr.h"
#include "nsCOMArray.h"
#include "nsIOutputStream.h"
#include "nsIPluginInstanceOwner.h"
#include "nsString.h"
@ -68,14 +68,49 @@ class nsINPAPIPluginStreamInfo : public nsIPluginStreamInfo
{
public:
NS_DECLARE_STATIC_IID_ACCESSOR(NS_INPAPIPLUGINSTREAMINFO_IID)
nsIRequest *GetRequest()
void TrackRequest(nsIRequest* request)
{
return mRequest;
mRequests.AppendObject(request);
}
void ReplaceRequest(nsIRequest* oldRequest, nsIRequest* newRequest)
{
PRInt32 i = mRequests.IndexOfObject(oldRequest);
if (i == -1) {
NS_ASSERTION(mRequests.Count() == 0,
"Only our initial stream should be unknown!");
mRequests.AppendObject(oldRequest);
}
else {
mRequests.ReplaceObjectAt(newRequest, i);
}
}
void CancelRequests(nsresult status)
{
// Copy the array to avoid modification during the loop.
nsCOMArray<nsIRequest> requestsCopy(mRequests);
for (PRInt32 i = 0; i < requestsCopy.Count(); ++i)
requestsCopy[i]->Cancel(status);
}
void SuspendRequests() {
nsCOMArray<nsIRequest> requestsCopy(mRequests);
for (PRInt32 i = 0; i < requestsCopy.Count(); ++i)
requestsCopy[i]->Suspend();
}
void ResumeRequests() {
nsCOMArray<nsIRequest> requestsCopy(mRequests);
for (PRInt32 i = 0; i < requestsCopy.Count(); ++i)
requestsCopy[i]->Resume();
}
protected:
nsCOMPtr<nsIRequest> mRequest;
friend class nsPluginByteRangeStreamListener;
nsCOMArray<nsIRequest> mRequests;
};
NS_DEFINE_STATIC_IID_ACCESSOR(nsINPAPIPluginStreamInfo,
@ -121,7 +156,7 @@ public:
nsresult CleanUpStream(NPReason reason);
void CallURLNotify(NPReason reason);
void SetCallNotify(PRBool aCallNotify) { mCallNotify = aCallNotify; }
nsresult SuspendRequest();
void SuspendRequest();
void ResumeRequest();
nsresult StartDataPump();
void StopDataPump();

View File

@ -3341,6 +3341,8 @@ nsresult nsPluginHost::NewPluginURLStream(const nsString& aURL,
rv = AddHeadersToChannel(aHeadersData, aHeadersDataLen, httpChannel);
}
rv = channel->AsyncOpen(listenerPeer, nsnull);
if (NS_SUCCEEDED(rv))
listenerPeer->TrackRequest(channel);
return rv;
}

View File

@ -51,6 +51,7 @@
#include "nsIURI.h"
#include "nsPluginHost.h"
#include "nsIByteRangeRequest.h"
#include "nsIMultiPartChannel.h"
#include "nsIInputStreamTee.h"
#include "nsPrintfCString.h"
#include "nsIContentUtils.h"
@ -62,7 +63,10 @@
// nsPluginByteRangeStreamListener
class nsPluginByteRangeStreamListener : public nsIStreamListener {
class nsPluginByteRangeStreamListener
: public nsIStreamListener
, public nsIInterfaceRequestor
{
public:
nsPluginByteRangeStreamListener(nsIWeakReference* aWeakPtr);
virtual ~nsPluginByteRangeStreamListener();
@ -70,6 +74,7 @@ public:
NS_DECL_ISUPPORTS
NS_DECL_NSIREQUESTOBSERVER
NS_DECL_NSISTREAMLISTENER
NS_DECL_NSIINTERFACEREQUESTOR
private:
nsCOMPtr<nsIStreamListener> mStreamConverter;
@ -77,7 +82,11 @@ private:
PRBool mRemoveMagicNumber;
};
NS_IMPL_ISUPPORTS1(nsPluginByteRangeStreamListener, nsIStreamListener)
NS_IMPL_ISUPPORTS3(nsPluginByteRangeStreamListener,
nsIRequestObserver,
nsIStreamListener,
nsIInterfaceRequestor)
nsPluginByteRangeStreamListener::nsPluginByteRangeStreamListener(nsIWeakReference* aWeakPtr)
{
mWeakPtrPluginStreamListenerPeer = aWeakPtr;
@ -90,6 +99,22 @@ nsPluginByteRangeStreamListener::~nsPluginByteRangeStreamListener()
mWeakPtrPluginStreamListenerPeer = 0;
}
/**
* Unwrap any byte-range requests so that we can check whether the base channel
* is being tracked properly.
*/
static nsCOMPtr<nsIRequest>
GetBaseRequest(nsIRequest* r)
{
nsCOMPtr<nsIMultiPartChannel> mp = do_QueryInterface(r);
if (!mp)
return r;
nsCOMPtr<nsIChannel> base;
mp->GetBaseChannel(getter_AddRefs(base));
return already_AddRefed<nsIRequest>(base.forget());
}
NS_IMETHODIMP
nsPluginByteRangeStreamListener::OnStartRequest(nsIRequest *request, nsISupports *ctxt)
{
@ -99,6 +124,12 @@ nsPluginByteRangeStreamListener::OnStartRequest(nsIRequest *request, nsISupports
if (!finalStreamListener)
return NS_ERROR_FAILURE;
nsPluginStreamListenerPeer *pslp =
static_cast<nsPluginStreamListenerPeer*>(finalStreamListener.get());
NS_ASSERTION(pslp->mRequests.IndexOfObject(GetBaseRequest(request)) != -1,
"Untracked byte-range request?");
nsCOMPtr<nsIStreamConverterService> serv = do_GetService(NS_STREAMCONVERTERSERVICE_CONTRACTID, &rv);
if (NS_SUCCEEDED(rv)) {
rv = serv->AsyncConvertData(MULTIPART_BYTERANGES,
@ -125,10 +156,6 @@ nsPluginByteRangeStreamListener::OnStartRequest(nsIRequest *request, nsISupports
return NS_ERROR_FAILURE;
}
// get nsPluginStreamListenerPeer* ptr from finalStreamListener
nsPluginStreamListenerPeer *pslp =
reinterpret_cast<nsPluginStreamListenerPeer*>(finalStreamListener.get());
if (responseCode != 200) {
PRBool bWantsAllNetworkStreams = PR_FALSE;
pslp->GetPluginInstance()->
@ -159,6 +186,13 @@ nsPluginByteRangeStreamListener::OnStopRequest(nsIRequest *request, nsISupports
if (!finalStreamListener)
return NS_ERROR_FAILURE;
nsPluginStreamListenerPeer *pslp =
static_cast<nsPluginStreamListenerPeer*>(finalStreamListener.get());
PRBool found = pslp->mRequests.RemoveObject(request);
if (!found) {
NS_ERROR("OnStopRequest received for untracked byte-range request!");
}
if (mRemoveMagicNumber) {
// remove magic number from container
nsCOMPtr<nsISupportsPRUint32> container = do_QueryInterface(ctxt);
@ -222,6 +256,18 @@ nsPluginByteRangeStreamListener::OnDataAvailable(nsIRequest *request, nsISupport
return mStreamConverter->OnDataAvailable(request, ctxt, inStr, sourceOffset, count);
}
NS_IMETHODIMP
nsPluginByteRangeStreamListener::GetInterface(const nsIID& aIID, void** result)
{
// Forward interface requests to our parent
nsCOMPtr<nsIInterfaceRequestor> finalStreamListener = do_QueryReferent(mWeakPtrPluginStreamListenerPeer);
if (!finalStreamListener)
return NS_ERROR_FAILURE;
return finalStreamListener->GetInterface(aIID, result);
}
// nsPRUintKey
class nsPRUintKey : public nsHashKey {
@ -297,8 +343,7 @@ nsPluginStreamListenerPeer::~nsPluginStreamListenerPeer()
// Called as a result of GetURL and PostURL
nsresult nsPluginStreamListenerPeer::Initialize(nsIURI *aURL,
nsNPAPIPluginInstance *aInstance,
nsIPluginStreamListener* aListener,
PRInt32 requestCount)
nsIPluginStreamListener* aListener)
{
#ifdef PLUGIN_LOGGING
nsCAutoString urlSpec;
@ -317,7 +362,7 @@ nsresult nsPluginStreamListenerPeer::Initialize(nsIURI *aURL,
mPStreamListener = static_cast<nsNPAPIPluginStreamListener*>(aListener);
mPStreamListener->SetStreamListenerPeer(this);
mPendingRequests = requestCount;
mPendingRequests = 1;
mDataForwardToRequest = new nsHashtable(16, PR_FALSE);
if (!mDataForwardToRequest)
@ -471,6 +516,12 @@ nsPluginStreamListenerPeer::OnStartRequest(nsIRequest *request,
nsISupports* aContext)
{
nsresult rv = NS_OK;
if (mRequests.IndexOfObject(GetBaseRequest(request)) == -1) {
NS_ASSERTION(mRequests.Count() == 0,
"Only our initial stream should be unknown!");
TrackRequest(request);
}
if (mHaveFiredOnStartRequest) {
return NS_OK;
@ -550,8 +601,6 @@ nsPluginStreamListenerPeer::OnStartRequest(nsIRequest *request,
mLength = length;
}
mRequest = request;
nsCAutoString aContentType; // XXX but we already got the type above!
rv = channel->GetContentType(aContentType);
if (NS_FAILED(rv))
@ -775,7 +824,10 @@ nsPluginStreamListenerPeer::RequestRead(NPByteRange* rangeList)
if (NS_FAILED(rv))
return rv;
return channel->AsyncOpen(converter, container);
rv = channel->AsyncOpen(converter, container);
if (NS_SUCCEEDED(rv))
TrackRequest(channel);
return rv;
}
NS_IMETHODIMP
@ -864,6 +916,9 @@ NS_IMETHODIMP nsPluginStreamListenerPeer::OnDataAvailable(nsIRequest *request,
PRUint32 sourceOffset,
PRUint32 aLength)
{
NS_ASSERTION(mRequests.IndexOfObject(GetBaseRequest(request)) != -1,
"Received OnDataAvailable for untracked request.");
if (mRequestFailed)
return NS_ERROR_FAILURE;
@ -885,8 +940,6 @@ NS_IMETHODIMP nsPluginStreamListenerPeer::OnDataAvailable(nsIRequest *request,
if (!mPStreamListener)
return NS_ERROR_FAILURE;
mRequest = request;
const char * url = nsnull;
GetURL(&url);
@ -968,6 +1021,14 @@ NS_IMETHODIMP nsPluginStreamListenerPeer::OnStopRequest(nsIRequest *request,
nsresult aStatus)
{
nsresult rv = NS_OK;
nsCOMPtr<nsIMultiPartChannel> mp = do_QueryInterface(request);
if (!mp) {
PRBool found = mRequests.RemoveObject(request);
if (!found) {
NS_ERROR("Received OnStopRequest for untracked request.");
}
}
PLUGIN_LOG(PLUGIN_LOG_NOISY,
("nsPluginStreamListenerPeer::OnStopRequest this=%p aStatus=%d request=%p\n",
@ -1073,7 +1134,6 @@ NS_IMETHODIMP nsPluginStreamListenerPeer::OnStopRequest(nsIRequest *request,
if (NS_SUCCEEDED(aStatus)) {
mStreamComplete = PR_TRUE;
}
mRequest = NULL;
return NS_OK;
}
@ -1284,6 +1344,47 @@ nsPluginStreamListenerPeer::GetInterface(const nsIID& aIID, void** result)
return GetInterfaceGlobal(aIID, result);
}
/**
* Proxy class which forwards async redirect notifications back to the necko
* callback, keeping nsPluginStreamListenerPeer::mRequests in sync with
* which channel is active.
*/
class ChannelRedirectProxyCallback : public nsIAsyncVerifyRedirectCallback
{
public:
ChannelRedirectProxyCallback(nsINPAPIPluginStreamInfo* listener,
nsIAsyncVerifyRedirectCallback* parent,
nsIChannel* oldChannel,
nsIChannel* newChannel)
: mWeakListener(do_GetWeakReference(listener))
, mParent(parent)
, mOldChannel(oldChannel)
, mNewChannel(newChannel)
{
}
NS_DECL_ISUPPORTS
NS_IMETHODIMP OnRedirectVerifyCallback(nsresult result)
{
if (NS_SUCCEEDED(result)) {
nsCOMPtr<nsINPAPIPluginStreamInfo> listener = do_QueryReferent(mWeakListener);
if (listener)
listener->ReplaceRequest(mOldChannel, mNewChannel);
}
return mParent->OnRedirectVerifyCallback(result);
}
private:
nsWeakPtr mWeakListener;
nsCOMPtr<nsIAsyncVerifyRedirectCallback> mParent;
nsCOMPtr<nsIChannel> mOldChannel;
nsCOMPtr<nsIChannel> mNewChannel;
};
NS_IMPL_ISUPPORTS1(ChannelRedirectProxyCallback, nsIAsyncVerifyRedirectCallback)
NS_IMETHODIMP
nsPluginStreamListenerPeer::AsyncOnChannelRedirect(nsIChannel *oldChannel, nsIChannel *newChannel,
PRUint32 flags, nsIAsyncVerifyRedirectCallback* callback)
@ -1293,8 +1394,11 @@ nsPluginStreamListenerPeer::AsyncOnChannelRedirect(nsIChannel *oldChannel, nsICh
return NS_ERROR_FAILURE;
}
nsCOMPtr<nsIAsyncVerifyRedirectCallback> proxyCallback =
new ChannelRedirectProxyCallback(this, callback, oldChannel, newChannel);
// Give NPAPI a chance to control redirects.
bool notificationHandled = mPStreamListener->HandleRedirectNotification(oldChannel, newChannel, callback);
bool notificationHandled = mPStreamListener->HandleRedirectNotification(oldChannel, newChannel, proxyCallback);
if (notificationHandled) {
return NS_OK;
}
@ -1331,5 +1435,5 @@ nsPluginStreamListenerPeer::AsyncOnChannelRedirect(nsIChannel *oldChannel, nsICh
return rv;
}
return channelEventSink->AsyncOnChannelRedirect(oldChannel, newChannel, flags, callback);
return channelEventSink->AsyncOnChannelRedirect(oldChannel, newChannel, flags, proxyCallback);
}

View File

@ -109,15 +109,14 @@ public:
// Called by GetURL and PostURL (via NewStream)
nsresult Initialize(nsIURI *aURL,
nsNPAPIPluginInstance *aInstance,
nsIPluginStreamListener *aListener,
PRInt32 requestCount = 1);
nsIPluginStreamListener *aListener);
nsresult InitializeEmbedded(nsIURI *aURL,
nsNPAPIPluginInstance* aInstance,
nsIPluginInstanceOwner *aOwner = nsnull);
nsresult InitializeFullPage(nsIURI* aURL, nsNPAPIPluginInstance *aInstance);
nsresult OnFileAvailable(nsIFile* aFile);
nsresult ServeStreamAsFile(nsIRequest *request, nsISupports *ctxt);