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 · 54270b33403801f8c831c9aec2f2f284661f8d25 · 54270b3
2 parents eee3dba + 6b2a605
commit 54270b33403801f8c831c9aec2f2f284661f8d25
Unified Split

Showing with 79 additions and 18 deletions.

+79 −18 core/src/main/java/hudson/model/UpdateCenter.java
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 boolean requiresRestart() {
            }
        }
+  
+        /**
+  
+         * 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;
            }
        }