Bug 719180: Part 1 - Correct jar channel stream ownership; r=taras

Avoid potential file locking issues by not keeping references to the input
stream in the first place, analog to how nsBaseChannel/nsFileChannel are
implemented. The file locks will now be released as soon as either the stream
is explictly closed or the stream instance gets destroyed.
This means that a synchronous ::Open() will transfer the input stream
ownership to the caller, while asynchronous ::AsyncOpen() will transfer it to
the pump handling the asynchronous transfer to the caller's listener.
This commit is contained in:
Nils Maier 2012-11-28 13:12:56 -05:00
parent 664dca8c89
commit bd5c1b8f5c
2 changed files with 95 additions and 94 deletions

View File

@ -57,8 +57,8 @@ public:
nsJARInputThunk(nsIZipReader *zipReader,
nsIURI* fullJarURI,
const nsACString &jarEntry,
nsIZipReaderCache *jarCache)
: mJarCache(jarCache)
bool usingJarCache)
: mUsingJarCache(usingJarCache)
, mJarReader(zipReader)
, mJarEntry(jarEntry)
, mContentLength(-1)
@ -74,13 +74,7 @@ public:
virtual ~nsJARInputThunk()
{
if (!mJarCache && mJarReader)
mJarReader->Close();
}
void GetJarReader(nsIZipReader **result)
{
NS_IF_ADDREF(*result = mJarReader);
Close();
}
int64_t GetContentLength()
@ -88,11 +82,11 @@ public:
return mContentLength;
}
nsresult EnsureJarStream();
nsresult Init();
private:
nsCOMPtr<nsIZipReaderCache> mJarCache;
bool mUsingJarCache;
nsCOMPtr<nsIZipReader> mJarReader;
nsCString mJarDirSpec;
nsCOMPtr<nsIInputStream> mJarStream;
@ -103,11 +97,8 @@ private:
NS_IMPL_THREADSAFE_ISUPPORTS1(nsJARInputThunk, nsIInputStream)
nsresult
nsJARInputThunk::EnsureJarStream()
nsJARInputThunk::Init()
{
if (mJarStream)
return NS_OK;
nsresult rv;
if (ENTRY_IS_DIRECTORY(mJarEntry)) {
// A directory stream also needs the Spec of the FullJarURI
@ -144,27 +135,28 @@ nsJARInputThunk::EnsureJarStream()
NS_IMETHODIMP
nsJARInputThunk::Close()
{
if (mJarStream)
return mJarStream->Close();
nsresult rv = NS_OK;
return NS_OK;
if (mJarStream)
rv = mJarStream->Close();
if (!mUsingJarCache && mJarReader)
mJarReader->Close();
mJarReader = nullptr;
return rv;
}
NS_IMETHODIMP
nsJARInputThunk::Available(uint64_t *avail)
{
nsresult rv = EnsureJarStream();
if (NS_FAILED(rv)) return rv;
return mJarStream->Available(avail);
}
NS_IMETHODIMP
nsJARInputThunk::Read(char *buf, uint32_t count, uint32_t *countRead)
{
nsresult rv = EnsureJarStream();
if (NS_FAILED(rv)) return rv;
return mJarStream->Read(buf, count, countRead);
}
@ -196,7 +188,6 @@ nsJARChannel::nsJARChannel()
, mStatus(NS_OK)
, mIsPending(false)
, mIsUnsafe(true)
, mJarInput(nullptr)
{
#if defined(PR_LOGGING)
if (!gJarProtocolLog)
@ -209,9 +200,6 @@ nsJARChannel::nsJARChannel()
nsJARChannel::~nsJARChannel()
{
// with the exception of certain error cases mJarInput will already be null.
NS_IF_RELEASE(mJarInput);
// release owning reference to the jar handler
nsJARProtocolHandler *handler = gJarHandler;
NS_RELEASE(handler); // NULL parameter
@ -261,8 +249,10 @@ nsJARChannel::Init(nsIURI *uri)
}
nsresult
nsJARChannel::CreateJarInput(nsIZipReaderCache *jarCache)
nsJARChannel::CreateJarInput(nsIZipReaderCache *jarCache, nsJARInputThunk **resultInput)
{
MOZ_ASSERT(resultInput);
// important to pass a clone of the file since the nsIFile impl is not
// necessarily MT-safe
nsCOMPtr<nsIFile> clonedFile;
@ -274,7 +264,7 @@ nsJARChannel::CreateJarInput(nsIZipReaderCache *jarCache)
if (jarCache) {
if (mInnerJarEntry.IsEmpty())
rv = jarCache->GetZip(mJarFile, getter_AddRefs(reader));
else
else
rv = jarCache->GetInnerZip(mJarFile, mInnerJarEntry,
getter_AddRefs(reader));
} else {
@ -300,26 +290,37 @@ nsJARChannel::CreateJarInput(nsIZipReaderCache *jarCache)
if (NS_FAILED(rv))
return rv;
mJarInput = new nsJARInputThunk(reader, mJarURI, mJarEntry, jarCache);
if (!mJarInput)
return NS_ERROR_OUT_OF_MEMORY;
NS_ADDREF(mJarInput);
nsRefPtr<nsJARInputThunk> input = new nsJARInputThunk(reader,
mJarURI,
mJarEntry,
jarCache != nullptr
);
rv = input->Init();
if (NS_FAILED(rv))
return rv;
// Make GetContentLength meaningful
mContentLength = input->GetContentLength();
input.forget(resultInput);
return NS_OK;
}
nsresult
nsJARChannel::EnsureJarInput(bool blocking)
nsJARChannel::LookupFile()
{
LOG(("nsJARChannel::EnsureJarInput [this=%x %s]\n", this, mSpec.get()));
LOG(("nsJARChannel::LookupFile [this=%x %s]\n", this, mSpec.get()));
nsresult rv;
nsCOMPtr<nsIURI> uri;
rv = mJarURI->GetJARFile(getter_AddRefs(mJarBaseURI));
if (NS_FAILED(rv)) return rv;
if (NS_FAILED(rv))
return rv;
rv = mJarURI->GetJAREntry(mJarEntry);
if (NS_FAILED(rv)) return rv;
if (NS_FAILED(rv))
return rv;
// The name of the JAR entry must not contain URL-escaped characters:
// we're moving from URL domain to a filename domain here. nsStandardURL
@ -349,27 +350,7 @@ nsJARChannel::EnsureJarInput(bool blocking)
}
}
if (mJarFile) {
mIsUnsafe = false;
// NOTE: we do not need to deal with mSecurityInfo here,
// because we're loading from a local file
rv = CreateJarInput(gJarHandler->JarCache());
}
else if (blocking) {
NS_NOTREACHED("need sync downloader");
rv = NS_ERROR_NOT_IMPLEMENTED;
}
else {
// kick off an async download of the base URI...
rv = NS_NewDownloader(getter_AddRefs(mDownloader), this);
if (NS_SUCCEEDED(rv))
rv = NS_OpenURI(mDownloader, nullptr, mJarBaseURI, nullptr,
mLoadGroup, mCallbacks,
mLoadFlags & ~(LOAD_DOCUMENT_URI | LOAD_CALL_CONTENT_SNIFFERS));
}
return rv;
}
//-----------------------------------------------------------------------------
@ -641,10 +622,6 @@ nsJARChannel::GetContentDispositionHeader(nsACString &aContentDispositionHeader)
NS_IMETHODIMP
nsJARChannel::GetContentLength(int64_t *result)
{
// if content length is unknown, query mJarInput...
if (mContentLength < 0 && mJarInput)
mContentLength = mJarInput->GetContentLength();
*result = mContentLength;
return NS_OK;
}
@ -662,26 +639,31 @@ nsJARChannel::Open(nsIInputStream **stream)
{
LOG(("nsJARChannel::Open [this=%x]\n", this));
NS_ENSURE_TRUE(!mJarInput, NS_ERROR_IN_PROGRESS);
NS_ENSURE_TRUE(!mOpened, NS_ERROR_IN_PROGRESS);
NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS);
mJarFile = nullptr;
mIsUnsafe = true;
nsresult rv = EnsureJarInput(true);
if (NS_FAILED(rv)) return rv;
nsresult rv = LookupFile();
if (NS_FAILED(rv))
return rv;
if (!mJarInput)
return NS_ERROR_UNEXPECTED;
// If mJarInput was not set by LookupFile, the JAR is a remote jar.
if (!mJarFile) {
NS_NOTREACHED("need sync downloader");
return NS_ERROR_NOT_IMPLEMENTED;
}
// force load the jar file now so GetContentLength will return a
// meaningful value once we return.
rv = mJarInput->EnsureJarStream();
if (NS_FAILED(rv)) return rv;
NS_ADDREF(*stream = mJarInput);
nsCOMPtr<nsJARInputThunk> input;
rv = CreateJarInput(gJarHandler->JarCache(), getter_AddRefs(input));
if (NS_FAILED(rv))
return rv;
input.forget(stream);
mOpened = true;
// local files are always considered safe
mIsUnsafe = false;
return NS_OK;
}
@ -691,6 +673,7 @@ nsJARChannel::AsyncOpen(nsIStreamListener *listener, nsISupports *ctx)
LOG(("nsJARChannel::AsyncOpen [this=%x]\n", this));
NS_ENSURE_ARG_POINTER(listener);
NS_ENSURE_TRUE(!mOpened, NS_ERROR_IN_PROGRESS);
NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS);
mJarFile = nullptr;
@ -699,30 +682,49 @@ nsJARChannel::AsyncOpen(nsIStreamListener *listener, nsISupports *ctx)
// Initialize mProgressSink
NS_QueryNotificationCallbacks(mCallbacks, mLoadGroup, mProgressSink);
nsresult rv = EnsureJarInput(false);
if (NS_FAILED(rv)) return rv;
nsresult rv = LookupFile();
if (NS_FAILED(rv))
return rv;
// These variables must only be set if we're going to trigger an
// OnStartRequest, either from AsyncRead or OnDownloadComplete.
//
// That means: Do not add early return statements beyond this point!
mListener = listener;
mListenerContext = ctx;
mIsPending = true;
if (mJarInput) {
// create input stream pump and call AsyncRead as a block
rv = NS_NewInputStreamPump(getter_AddRefs(mPump), mJarInput);
if (NS_SUCCEEDED(rv))
rv = mPump->AsyncRead(this, nullptr);
// If we failed to create the pump or initiate the AsyncRead,
// then we need to clear these variables.
if (NS_FAILED(rv)) {
mIsPending = false;
mListenerContext = nullptr;
mListener = nullptr;
return rv;
if (!mJarFile) {
// Not a local file...
// kick off an async download of the base URI...
rv = NS_NewDownloader(getter_AddRefs(mDownloader), this);
if (NS_SUCCEEDED(rv))
rv = NS_OpenURI(mDownloader, nullptr, mJarBaseURI, nullptr,
mLoadGroup, mCallbacks,
mLoadFlags & ~(LOAD_DOCUMENT_URI | LOAD_CALL_CONTENT_SNIFFERS));
}
else {
// local files are always considered safe
mIsUnsafe = false;
nsCOMPtr<nsJARInputThunk> input;
rv = CreateJarInput(gJarHandler->JarCache(), getter_AddRefs(input));
if (NS_SUCCEEDED(rv)) {
// create input stream pump and call AsyncRead as a block
rv = NS_NewInputStreamPump(getter_AddRefs(mPump), input);
if (NS_SUCCEEDED(rv))
rv = mPump->AsyncRead(this, nullptr);
}
}
if (NS_FAILED(rv)) {
mIsPending = false;
mListenerContext = nullptr;
mListener = nullptr;
return rv;
}
if (mLoadGroup)
mLoadGroup->AddRequest(this, nullptr);
@ -842,11 +844,12 @@ nsJARChannel::OnDownloadComplete(nsIDownloader *downloader,
if (NS_SUCCEEDED(status)) {
mJarFile = file;
rv = CreateJarInput(nullptr);
nsCOMPtr<nsJARInputThunk> input;
rv = CreateJarInput(nullptr, getter_AddRefs(input));
if (NS_SUCCEEDED(rv)) {
// create input stream pump
rv = NS_NewInputStreamPump(getter_AddRefs(mPump), mJarInput);
rv = NS_NewInputStreamPump(getter_AddRefs(mPump), input);
if (NS_SUCCEEDED(rv))
rv = mPump->AsyncRead(this, nullptr);
}
@ -893,7 +896,6 @@ nsJARChannel::OnStopRequest(nsIRequest *req, nsISupports *ctx, nsresult status)
mLoadGroup->RemoveRequest(this, nullptr, status);
mPump = 0;
NS_IF_RELEASE(mJarInput);
mIsPending = false;
mDownloader = 0; // this may delete the underlying jar file

View File

@ -46,8 +46,8 @@ public:
nsresult Init(nsIURI *uri);
private:
nsresult CreateJarInput(nsIZipReaderCache *);
nsresult EnsureJarInput(bool blocking);
nsresult CreateJarInput(nsIZipReaderCache *, nsJARInputThunk **);
nsresult LookupFile();
#if defined(PR_LOGGING)
nsCString mSpec;
@ -77,7 +77,6 @@ private:
bool mIsPending;
bool mIsUnsafe;
nsJARInputThunk *mJarInput;
nsCOMPtr<nsIStreamListener> mDownloader;
nsCOMPtr<nsIInputStreamPump> mPump;
nsCOMPtr<nsIFile> mJarFile;