Merge pull request #2258 from kzantow/JENKINS-33950-multiple-dependen…

…cy-install-fix [JENKINS-33950] dependencies may be installed multiple times
jenkinsci · May 1, 2016 · 54270b3 · 54270b3
2 parents eee3dba + 6b2a605
commit 54270b3
Showing 1 changed file with 79 additions and 18 deletions.
diff --git a/core/src/main/java/hudson/model/UpdateCenter.java b/core/src/main/java/hudson/model/UpdateCenter.java
@@ -1619,6 +1619,15 @@ public class Success extends InstallationStatus {
             }
         }
 
+        /**
+         * Indicates that the plugin was successfully installed.
+         */
+        public class Skipped extends InstallationStatus {
+            @Override public boolean isSuccess() {
+                return true;
+            }
+        }
+
         /**
          * Indicates that the plugin is waiting for its turn for installation.
          */
@@ -1711,29 +1720,81 @@ public String getName() {
 
         @Override
         public void _run() throws IOException, InstallationStatus {
-            super._run();
+            if (wasInstalled()) {
+                // Do this first so we can avoid duplicate downloads, too
+                // check to see if the plugin is already installed at the same version and skip it
+                LOGGER.warning("Skipping duplicate install of: " + plugin.getDisplayName() + "@" + plugin.version);
+                //throw new Skipped(); // TODO set skipped once we have a status indicator for it
+                return;
+            }
+            try {
+                super._run();
+
+                // if this is a bundled plugin, make sure it won't get overwritten
+                PluginWrapper pw = plugin.getInstalled();
+                if (pw!=null && pw.isBundled()) {
+                    SecurityContext oldContext = ACL.impersonate(ACL.SYSTEM);
+                    try {
+                        pw.doPin();
+                    } finally {
+                        SecurityContextHolder.setContext(oldContext);
+                    }
+                }
 
-            // if this is a bundled plugin, make sure it won't get overwritten
-            PluginWrapper pw = plugin.getInstalled();
-            if (pw!=null && pw.isBundled()) {
-                SecurityContext oldContext = ACL.impersonate(ACL.SYSTEM);
-                try {
-                    pw.doPin();
-                } finally {
-                    SecurityContextHolder.setContext(oldContext);
+                if (dynamicLoad) {
+                    try {
+                        pm.dynamicLoad(getDestination());
+                    } catch (RestartRequiredException e) {
+                        throw new SuccessButRequiresRestart(e.message);
+                    } catch (Exception e) {
+                        throw new IOException("Failed to dynamically deploy this plugin",e);
+                    }
+                } else {
+                    throw new SuccessButRequiresRestart(Messages._UpdateCenter_DownloadButNotActivated());
+                }
+            } finally {
+                synchronized(this) {
+                    // There may be other threads waiting on completion
+                    LOGGER.fine("Install complete for: " + plugin.getDisplayName() + "@" + plugin.version);
+                    // some status other than Installing or Downloading needs to be set here
+                    // {@link #isAlreadyInstalling()}, it will be overwritten by {@link DownloadJob#run()}
+                    status = new Skipped();
+                    notifyAll();
                 }
             }
+        }
 
-            if (dynamicLoad) {
-                try {
-                    pm.dynamicLoad(getDestination());
-                } catch (RestartRequiredException e) {
-                    throw new SuccessButRequiresRestart(e.message);
-                } catch (Exception e) {
-                    throw new IOException("Failed to dynamically deploy this plugin",e);
+        /**
+         * Indicates there is another installation job for this plugin
+         * @since TODO
+         */
+        protected boolean wasInstalled() {
+            synchronized(UpdateCenter.this) {
+                for (UpdateCenterJob job : getJobs()) {
+                    if (job == this) {
+                        // oldest entries first, if we reach this instance,
+                        // we need it to continue installing
+                        return false;
+                    }
+                    if (job instanceof InstallationJob) {
+                        InstallationJob ij = (InstallationJob)job;
+                        if (ij.plugin.equals(plugin) && ij.plugin.version.equals(plugin.version)) {
+                            // wait until other install is completed
+                            synchronized(ij) {
+                                if(ij.status instanceof Installing || ij.status instanceof Pending) {
+                                    try {
+                                        LOGGER.fine("Waiting for other plugin install of: " + plugin.getDisplayName() + "@" + plugin.version);
+                                        ij.wait();
+                                    } catch (InterruptedException e) {
+                                        throw new RuntimeException(e);
+                                    }
+                                }
+                            }
+                            return true;
+                        }
+                    }
                 }
-            } else {
-                throw new SuccessButRequiresRestart(Messages._UpdateCenter_DownloadButNotActivated());
+                return false;
             }
         }