Bug 1048615 - Fix crash when simultaneously opening remote JAR file with mEnsureChildFd enabled. r=jduell
authorShian-Yow Wu <swu@mozilla.com>
Thu, 21 Aug 2014 19:09:35 +0800
changeset 222510 f3c7ef477d3d142a5bbace0af508dbecd5f5dcaa
parent 222509 0bd4d6c736e178d516215d960fc59d5805960b16
child 222511 b7529dcda6eb9675b0e207e0130efa77ecda4b47
push id3979
push userraliiev@mozilla.com
push dateMon, 13 Oct 2014 16:35:44 +0000
treeherdermozilla-beta@30f2cc610691 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjduell
bugs1048615
milestone34.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
Bug 1048615 - Fix crash when simultaneously opening remote JAR file with mEnsureChildFd enabled. r=jduell
modules/libjar/nsJARChannel.cpp
modules/libjar/nsJARChannel.h
modules/libjar/test/unit/test_jarchannel.js
--- a/modules/libjar/nsJARChannel.cpp
+++ b/modules/libjar/nsJARChannel.cpp
@@ -376,41 +376,38 @@ nsJARChannel::LookupFile()
                     return NS_OK;
                     #else
                     if (!mEnsureChildFd) {
                         return NS_OK;
                     }
                     PRFileDesc *fd = nullptr;
                     jarCache->GetFd(mJarFile, &fd);
                     if (fd) {
-                        PROsfd osfd = dup(PR_FileDesc2NativeHandle(fd));
-                        if (osfd == -1) {
-                            return NS_ERROR_FAILURE;
-                        }
-                        remoteFile->SetNSPRFileDesc(PR_ImportFile(osfd));
-                        return NS_OK;
+                        return SetRemoteNSPRFileDesc(fd);
                     }
                     #endif
                 }
             }
 
             mOpeningRemote = true;
 
-            if (gJarHandler->RemoteOpenFileInProgress(remoteFile, this) &&
-                !mEnsureChildFd) {
+            #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
+            #else
+            if (mEnsureChildFd && jarCache) {
+                jarCache->SetMustCacheFd(remoteFile, true);
+            }
+            #endif
+
+            if (gJarHandler->RemoteOpenFileInProgress(remoteFile, this)) {
                 // JarHandler will trigger OnRemoteFileOpen() after the first
                 // request for this file completes and we'll get a JAR cache
                 // hit.
                 return NS_OK;
             }
 
-            if (mEnsureChildFd && jarCache) {
-                jarCache->SetMustCacheFd(remoteFile, true);
-            }
-
             // Open file on parent: OnRemoteFileOpenComplete called when done
             nsCOMPtr<nsITabChild> tabChild;
             NS_QueryNotificationCallbacks(this, tabChild);
             nsCOMPtr<nsILoadContext> loadContext;
             NS_QueryNotificationCallbacks(this, loadContext);
             rv = remoteFile->AsyncRemoteFileOpen(PR_RDONLY, this, tabChild,
                                                  loadContext);
             NS_ENSURE_SUCCESS(rv, rv);
@@ -472,16 +469,34 @@ nsJARChannel::FireOnProgress(uint64_t aP
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mProgressSink);
 
   mProgressSink->OnProgress(this, nullptr, aProgress,
                             uint64_t(mContentLength));
 }
 
+nsresult
+nsJARChannel::SetRemoteNSPRFileDesc(PRFileDesc *fd)
+{
+    PROsfd osfd = dup(PR_FileDesc2NativeHandle(fd));
+    if (osfd == -1) {
+        return NS_ERROR_FAILURE;
+    }
+
+    RemoteOpenFileChild* remoteFile =
+        static_cast<RemoteOpenFileChild*>(mJarFile.get());
+    nsresult rv = remoteFile->SetNSPRFileDesc(PR_ImportFile(osfd));
+    if (NS_FAILED(rv)) {
+        close(osfd);
+    }
+
+    return rv;
+}
+
 //-----------------------------------------------------------------------------
 // nsIRequest
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 nsJARChannel::GetName(nsACString &result)
 {
     return mJarURI->GetSpec(result);
@@ -1030,17 +1045,42 @@ nsJARChannel::OnDownloadComplete(nsIDown
 nsresult
 nsJARChannel::OnRemoteFileOpenComplete(nsresult aOpenStatus)
 {
     nsresult rv = aOpenStatus;
 
     // NS_ERROR_ALREADY_OPENED here means we'll hit JAR cache in
     // OpenLocalFile().
     if (NS_SUCCEEDED(rv) || rv == NS_ERROR_ALREADY_OPENED) {
-        rv = OpenLocalFile();
+        #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
+        // Windows/OSX desktop builds skip remoting, we don't need file
+        // descriptor here.
+        #else
+        if (mEnsureChildFd) {
+            // Set file descriptor from Jar cache into remote Jar file, if it
+            // has not been set previously.
+            mozilla::AutoFDClose fd;
+            mJarFile->OpenNSPRFileDesc(PR_RDONLY, 0, &fd.rwget());
+            if (!fd) {
+                nsIZipReaderCache *jarCache = gJarHandler->JarCache();
+                if (!jarCache) {
+                    rv = NS_ERROR_FAILURE;
+                }
+                PRFileDesc *jar_fd = nullptr;
+                jarCache->GetFd(mJarFile, &jar_fd);
+                // If we failed to get fd here, an error rv would be returned
+                // by SetRemoteNSPRFileDesc(), which would then stop the
+                // channel by NotifyError().
+                rv = SetRemoteNSPRFileDesc(jar_fd);
+            }
+        }
+        #endif
+        if (NS_SUCCEEDED(rv) || rv == NS_ERROR_ALREADY_OPENED) {
+            rv = OpenLocalFile();
+        }
     }
 
     if (NS_FAILED(rv)) {
         NotifyError(rv);
     }
 
     return NS_OK;
 }
@@ -1082,24 +1122,27 @@ nsJARChannel::OnStopRequest(nsIRequest *
     mPump = 0;
     mIsPending = false;
     mDownloader = 0; // this may delete the underlying jar file
 
     // Drop notification callbacks to prevent cycles.
     mCallbacks = 0;
     mProgressSink = 0;
 
+    #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
+    #else
     if (mEnsureChildFd) {
       nsIZipReaderCache *jarCache = gJarHandler->JarCache();
       if (jarCache) {
           jarCache->SetMustCacheFd(mJarFile, false);
       }
       // To deallocate file descriptor by RemoteOpenFileChild destructor.
       mJarFile = nullptr;
     }
+    #endif
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsJARChannel::OnDataAvailable(nsIRequest *req, nsISupports *ctx,
                                nsIInputStream *stream,
                                uint64_t offset, uint32_t count)
--- a/modules/libjar/nsJARChannel.h
+++ b/modules/libjar/nsJARChannel.h
@@ -56,18 +56,18 @@ public:
 
 private:
     virtual ~nsJARChannel();
 
     nsresult CreateJarInput(nsIZipReaderCache *, nsJARInputThunk **);
     nsresult LookupFile();
     nsresult OpenLocalFile();
     void NotifyError(nsresult aError);
-
     void FireOnProgress(uint64_t aProgress);
+    nsresult SetRemoteNSPRFileDesc(PRFileDesc *fd);
 
 #if defined(PR_LOGGING)
     nsCString                       mSpec;
 #endif
 
     bool                            mOpened;
 
     nsCOMPtr<nsIJARURI>             mJarURI;
--- a/modules/libjar/test/unit/test_jarchannel.js
+++ b/modules/libjar/test/unit/test_jarchannel.js
@@ -53,18 +53,19 @@ Listener.prototype = {
         }
         catch (ex) {
             do_throw(ex);
         }
     },
     onStartRequest: function(request, ctx) {
         this.gotStartRequest = true;
     },
-    onStopRequest: function(request, ctx) {
+    onStopRequest: function(request, ctx, status) {
         this.gotStopRequest = true;
+        do_check_eq(status, 0);
         if (this._callback) {
             this._callback.call(null, this);
         }
     }
 };
 
 /**
  * Basic reading test for asynchronously opened jar channel
@@ -208,9 +209,46 @@ if (!inChild) {
           }
 
           run_next_test();
       }), null);
   });
 
 } // if !inChild
 
+if (inChild) {
+    /**
+     * Multiple simultaneous opening test for bug 1048615
+     */
+    add_test(function testSimultaneous() {
+        var uri = jarBase + "/inner1.zip";
+
+        // Drop any JAR caches
+        obs.notifyObservers(null, "chrome-flush-caches", null);
+
+        // Open the first channel without ensureChildFd()
+        var chan_first = ios.newChannel(uri, null, null)
+                            .QueryInterface(Ci.nsIJARChannel);
+        chan_first.asyncOpen(new Listener(function(l) {
+        }), null);
+
+        // Open multiple channels with ensureChildFd()
+        var num = 10;
+        var chan = [];
+        for (var i = 0; i < num; i++) {
+            chan[i] = ios.newChannel(uri, null, null)
+                         .QueryInterface(Ci.nsIJARChannel);
+            chan[i].ensureChildFd();
+            chan[i].asyncOpen(new Listener(function(l) {
+            }), null);
+        }
+
+        // Open the last channel with ensureChildFd()
+        var chan_last = ios.newChannel(uri, null, null)
+                           .QueryInterface(Ci.nsIJARChannel);
+        chan_last.ensureChildFd();
+        chan_last.asyncOpen(new Listener(function(l) {
+            run_next_test();
+        }), null);
+    });
+} // if inChild
+
 function run_test() run_next_test();