diff --git a/core/src/main/java/hudson/XmlFile.java b/core/src/main/java/hudson/XmlFile.java index 3c0a68bee3..d5077b34db 100644 --- a/core/src/main/java/hudson/XmlFile.java +++ b/core/src/main/java/hudson/XmlFile.java @@ -120,6 +120,7 @@ public final class XmlFile { private final XStream xs; private final File file; private static final Map beingWritten = Collections.synchronizedMap(new IdentityHashMap<>()); + private static final ThreadLocal writing = new ThreadLocal<>(); public XmlFile(File file) { this(DEFAULT_XSTREAM,file); @@ -175,10 +176,12 @@ public final class XmlFile { try { w.write("\n"); beingWritten.put(o, null); + writing.set(file); try { xs.toXML(o, w); } finally { beingWritten.remove(o); + writing.set(null); } w.commit(); } catch(StreamException e) { @@ -200,11 +203,11 @@ public final class XmlFile { * @since 2.74 */ public static Object replaceIfNotAtTopLevel(Object o, Supplier replacement) { - if (beingWritten.containsKey(o)) { + File currentlyWriting = writing.get(); + if (beingWritten.containsKey(o) || currentlyWriting == null) { return o; } else { - // Unfortunately we cannot easily tell which XML file is actually being saved here, at least without implementing a custom Converter. - LOGGER.log(Level.WARNING, "JENKINS-45892: reference to {0} being saved but not at top level", o); + LOGGER.log(Level.WARNING, "JENKINS-45892: reference to " + o + " being saved from unexpected " + currentlyWriting, new IllegalStateException()); return replacement.get(); } } diff --git a/test/src/test/java/hudson/model/AbstractItem2Test.java b/test/src/test/java/hudson/model/AbstractItem2Test.java index a35044ee87..968dc5f54f 100644 --- a/test/src/test/java/hudson/model/AbstractItem2Test.java +++ b/test/src/test/java/hudson/model/AbstractItem2Test.java @@ -23,6 +23,8 @@ */ package hudson.model; +import hudson.XmlFile; +import java.util.logging.Level; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; import org.junit.Test; @@ -30,6 +32,7 @@ import static org.junit.Assert.*; import org.junit.Rule; import org.junit.runners.model.Statement; import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.LoggerRule; import org.jvnet.hudson.test.RestartableJenkinsRule; public class AbstractItem2Test { @@ -37,6 +40,9 @@ public class AbstractItem2Test { @Rule public RestartableJenkinsRule rr = new RestartableJenkinsRule(); + @Rule + public LoggerRule logging = new LoggerRule().record(XmlFile.class, Level.WARNING).capture(100); + @Issue("JENKINS-45892") @Test public void badSerialization() { @@ -50,6 +56,8 @@ public class AbstractItem2Test { String text = p2.getConfigFile().asString(); assertThat(text, not(containsString("this is p1"))); assertThat(text, containsString("p1")); + assertThat(logging.getMessages().toString(), containsString(p1.toString())); + assertThat(logging.getMessages().toString(), containsString(p2.getConfigFile().toString())); } }); rr.addStep(new Statement() {