Bug 1048615 - Fix crash when simultaneously opening remote JAR file with mEnsureChildFd enabled. r=jduell

modules/libjar/nsJARChannel.cpp

@@ -381,12 +381,7 @@ nsJARChannel::LookupFile()
                     PRFileDesc *fd = nullptr;
                     jarCache->GetFd(mJarFile, &fd);
                     if (fd) {
-                        PROsfd osfd = dup(PR_FileDesc2NativeHandle(fd));
+                        return SetRemoteNSPRFileDesc(fd);
-                        if (osfd == -1) {
-                            return NS_ERROR_FAILURE;
-                        }
-                        remoteFile->SetNSPRFileDesc(PR_ImportFile(osfd));
-                        return NS_OK;
                     }
                     #endif
                 }
@@ -394,18 +389,20 @@ nsJARChannel::LookupFile()
 
             mOpeningRemote = true;
 
-            if (gJarHandler->RemoteOpenFileInProgress(remoteFile, this) &&
+            #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
-                !mEnsureChildFd) {
+            #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);
@@ -477,6 +474,24 @@ nsJARChannel::FireOnProgress(uint64_t 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
 //-----------------------------------------------------------------------------
@@ -1035,7 +1050,32 @@ nsJARChannel::OnRemoteFileOpenComplete(nsresult 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)) {
@@ -1087,6 +1127,8 @@ nsJARChannel::OnStopRequest(nsIRequest *req, nsISupports *ctx, nsresult status)
     mCallbacks = 0;
     mProgressSink = 0;
 
+    #if defined(XP_WIN) || defined(MOZ_WIDGET_COCOA)
+    #else
     if (mEnsureChildFd) {
       nsIZipReaderCache *jarCache = gJarHandler->JarCache();
       if (jarCache) {
@@ -1095,6 +1137,7 @@ nsJARChannel::OnStopRequest(nsIRequest *req, nsISupports *ctx, nsresult status)
       // To deallocate file descriptor by RemoteOpenFileChild destructor.
       mJarFile = nullptr;
     }
+    #endif
 
     return NS_OK;
 }

modules/libjar/nsJARChannel.h

@@ -61,8 +61,8 @@ private:
     nsresult LookupFile();
     nsresult OpenLocalFile();
     void NotifyError(nsresult aError);
-
     void FireOnProgress(uint64_t aProgress);
+    nsresult SetRemoteNSPRFileDesc(PRFileDesc *fd);
 
 #if defined(PR_LOGGING)
     nsCString                       mSpec;

modules/libjar/test/unit/test_jarchannel.js

@@ -58,8 +58,9 @@ Listener.prototype = {
     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);
         }
@@ -213,4 +214,41 @@ if (!inChild) {
 
 } // 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();