Skip to content
Permalink
Browse files

[JENKINS-19544] Whether or not we manage to save an object with old d…

…ata, be sure to remove it from the list, and continue with other objects.

Otherwise a bug like JENKINS-20950 can prevent *anything* from being saved, and holds references to all the old data.
  • Loading branch information
jglick committed Mar 21, 2014
1 parent 10eca37 commit 8508fc365ac9faa4fa6ccee116e820c0455f0988
@@ -68,6 +68,10 @@
private HashMap<Saveable,VersionRange> data = new HashMap<Saveable,VersionRange>();
private boolean updating = false;

static OldDataMonitor get(Jenkins j) {
return (OldDataMonitor) j.getAdministrativeMonitor("OldData");
}

public OldDataMonitor() {
super("OldData");
}
@@ -86,7 +90,7 @@ public boolean isActivated() {
}

private static void remove(Saveable obj, boolean isDelete) {
OldDataMonitor odm = (OldDataMonitor) Jenkins.getInstance().getAdministrativeMonitor("OldData");
OldDataMonitor odm = get(Jenkins.getInstance());
synchronized (odm) {
if (odm.updating) return; // Skip during doUpgrade or doDiscard
odm.data.remove(obj);
@@ -130,7 +134,7 @@ public void onDeleted(Run run) {
* @param version Hudson release when the data structure changed.
*/
public static void report(Saveable obj, String version) {
OldDataMonitor odm = (OldDataMonitor) Jenkins.getInstance().getAdministrativeMonitor("OldData");
OldDataMonitor odm = get(Jenkins.getInstance());
synchronized (odm) {
try {
VersionRange vr = odm.data.get(obj);
@@ -184,7 +188,7 @@ public static void report(Saveable obj, Collection<Throwable> errors) {
}
return;
}
OldDataMonitor odm = (OldDataMonitor) j.getAdministrativeMonitor("OldData");
OldDataMonitor odm = get(j);
synchronized (odm) {
VersionRange vr = odm.data.get(obj);
if (vr != null) vr.extra = buf.toString();
@@ -258,16 +262,20 @@ public HttpResponse doAct(StaplerRequest req, StaplerResponse rsp) throws IOExce
* Remove those items from the data map.
*/
@RequirePOST
public synchronized HttpResponse doUpgrade(StaplerRequest req, StaplerResponse rsp) throws IOException {
public synchronized HttpResponse doUpgrade(StaplerRequest req, StaplerResponse rsp) {
String thruVerParam = req.getParameter("thruVer");
VersionNumber thruVer = thruVerParam.equals("all") ? null : new VersionNumber(thruVerParam);
updating = true;
for (Iterator<Map.Entry<Saveable,VersionRange>> it = data.entrySet().iterator(); it.hasNext();) {
Map.Entry<Saveable,VersionRange> entry = it.next();
VersionNumber version = entry.getValue().max;
if (version != null && (thruVer == null || !version.isNewerThan(thruVer))) {
entry.getKey().save();
it.remove();
try {
entry.getKey().save();
} catch (Exception x) {
LOGGER.log(Level.WARNING, "failed to save " + entry.getKey(), x);
}
}
}
updating = false;
@@ -279,13 +287,17 @@ public synchronized HttpResponse doUpgrade(StaplerRequest req, StaplerResponse r
* Remove those items from the data map.
*/
@RequirePOST
public synchronized HttpResponse doDiscard(StaplerRequest req, StaplerResponse rsp) throws IOException {
public synchronized HttpResponse doDiscard(StaplerRequest req, StaplerResponse rsp) {
updating = true;
for (Iterator<Map.Entry<Saveable,VersionRange>> it = data.entrySet().iterator(); it.hasNext();) {
Map.Entry<Saveable,VersionRange> entry = it.next();
if (entry.getValue().max == null) {
entry.getKey().save();
it.remove();
try {
entry.getKey().save();
} catch (Exception x) {
LOGGER.log(Level.WARNING, "failed to save " + entry.getKey(), x);
}
}
}
updating = false;
@@ -0,0 +1,67 @@
/*
* The MIT License
*
* Copyright 2014 Jesse Glick.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package hudson.diagnosis;

import hudson.model.FreeStyleProject;
import hudson.model.InvisibleAction;
import java.util.Collections;
import static org.junit.Assert.*;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Bug;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.recipes.LocalData;

public class OldDataMonitorTest {

@Rule public JenkinsRule r = new JenkinsRule();

@Bug(19544)
@LocalData
@Test public void robustness() throws Exception {
OldDataMonitor odm = OldDataMonitor.get(r.jenkins);
FreeStyleProject p = r.jenkins.getItemByFullName("busted", FreeStyleProject.class);
assertNotNull(p);
/*
System.err.println(p.getActions());
for (Map.Entry<Saveable,OldDataMonitor.VersionRange> entry : odm.getData().entrySet()) {
System.err.println(entry.getKey());
System.err.println(entry.getValue());
System.err.println(entry.getValue().extra);
}
*/
assertEquals(Collections.singleton(p), odm.getData().keySet());
odm.doDiscard(null, null);
assertEquals(Collections.emptySet(), odm.getData().keySet());
// did not manage to save p, but at least we are not holding onto a reference to it anymore
}

public static final class BadAction extends InvisibleAction {
private Object writeReplace() {
throw new IllegalStateException("broken");
}
}

}
Binary file not shown.

0 comments on commit 8508fc3

Please sign in to comment.
You can’t perform that action at this time.