Skip to content

Commit

Permalink
Merge pull request #838 from jglick/java.io.tmpdir
Browse files Browse the repository at this point in the history 
Improved use of `java.io.tmpdir`
  • Loading branch information
@jglick
jglick committed Sep 19, 2024
2 parents 53b76bc + f3f72c7 commit 9e8bb87
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 35 deletions.
30 changes: 23 additions & 7 deletions src/main/java/org/jvnet/hudson/test/InboundAgentRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,17 @@
import java.io.IOException;
import java.io.Serializable;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.jar.JarFile;
import java.util.logging.Level;
Expand Down Expand Up @@ -83,6 +87,7 @@ public final class InboundAgentRule extends ExternalResource {

private final String id = UUID.randomUUID().toString();
private final Map<String, Process> procs = Collections.synchronizedMap(new HashMap<>());
private final Set<String> workDirs = Collections.synchronizedSet(new HashSet<>());

/**
* The options used to (re)start an inbound agent.
Expand Down Expand Up @@ -278,6 +283,7 @@ public Slave createAgent(@NonNull JenkinsRule r, @CheckForNull String name) thro
*/
public Slave createAgent(@NonNull JenkinsRule r, Options options) throws Exception {
Slave s = createAgentJR(r, options);
workDirs.add(s.getRemoteFS());
if (options.isStart()) {
start(r, options);
}
Expand All @@ -289,8 +295,9 @@ public void createAgent(@NonNull RealJenkinsRule rr, @CheckForNull String name)
}

public void createAgent(@NonNull RealJenkinsRule rr, Options options) throws Throwable {
String name = rr.runRemotely(InboundAgentRule::createAgentRJR, options);
options.name = name;
var nameAndWorkDir = rr.runRemotely(InboundAgentRule::createAgentRJR, options);
options.name = nameAndWorkDir[0];
workDirs.add(nameAndWorkDir[1]);
if (options.isStart()) {
start(rr, options);
}
Expand Down Expand Up @@ -423,6 +430,7 @@ public boolean isAlive(String name) {
return proc != null && proc.isAlive();
}

@SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "test code")
@Override protected void after() {
for (var entry : procs.entrySet()) {
String name = entry.getKey();
Expand All @@ -440,6 +448,14 @@ public boolean isAlive(String name) {
}
}
procs.clear();
for (var workDir : workDirs) {
LOGGER.info(() -> "Deleting " + workDir);
try {
FileUtils.deleteDirectory(new File(workDir));
} catch (IOException x) {
LOGGER.log(Level.WARNING, null, x);
}
}
}

/**
Expand All @@ -465,7 +481,7 @@ private static AgentArguments getAgentArguments(JenkinsRule r, String name) thro
if (!launcher.getWorkDirSettings().isDisabled()) {
commandLineArgs = launcher.getWorkDirSettings().toCommandLineArgs(c);
}
File agentJar = new File(r.jenkins.getRootDir(), "agent.jar");
File agentJar = new File(System.getProperty("java.io.tmpdir"), "agent.jar");
if (!agentJar.isFile()) {
FileUtils.copyURLToFile(new Slave.JnlpJar("agent.jar").getURL(), agentJar);
}
Expand Down Expand Up @@ -497,9 +513,9 @@ private static void waitForAgentOffline(JenkinsRule r, String name) throws Inter
}
}

private static String createAgentRJR(JenkinsRule r, Options options) throws Throwable {
createAgentJR(r, options);
return options.getName();
private static String[] createAgentRJR(JenkinsRule r, Options options) throws Throwable {
var agent = createAgentJR(r, options);
return new String[] {options.getName(), agent.getRemoteFS()};
}

@SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "just for test code")
Expand All @@ -509,7 +525,7 @@ private static Slave createAgentJR(JenkinsRule r, Options options) throws Descri
}
JNLPLauncher launcher = new JNLPLauncher(options.getTunnel());
launcher.setWebSocket(options.isWebSocket());
DumbSlave s = new DumbSlave(options.getName(), new File(r.jenkins.getRootDir(), "agent-work-dirs/" + options.getName()).getAbsolutePath(), launcher);
DumbSlave s = new DumbSlave(options.getName(), Files.createTempDirectory(Path.of(System.getProperty("java.io.tmpdir")), options.getName() + "-work").toString(), launcher);
s.setLabelString(options.getLabel());
s.setRetentionStrategy(RetentionStrategy.NOOP);
r.jenkins.addNode(s);
Expand Down
12 changes: 11 additions & 1 deletion src/main/java/org/jvnet/hudson/test/JavaNetReverseProxy2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ public class JavaNetReverseProxy2 extends HttpServlet {
public JavaNetReverseProxy2(File cacheFolder) throws Exception {
this.cacheFolder = cacheFolder;
cacheFolder.mkdirs();
Runtime.getRuntime().addShutdownHook(new Thread("delete " + cacheFolder) {
@Override
public void run() {
try {
Util.deleteRecursive(cacheFolder);
} catch (IOException x) {
x.printStackTrace();
}
}
});

QueuedThreadPool qtp = new QueuedThreadPool();
qtp.setName("Jetty (JavaNetReverseProxy)");
server = new Server(qtp);
Expand Down Expand Up @@ -90,7 +101,6 @@ private String getMimeType(String path) {
*/
public static synchronized JavaNetReverseProxy2 getInstance() throws Exception {
if (INSTANCE == null) {
// TODO: think of a better location --- ideally inside the target/ dir so that clean would wipe them out
INSTANCE = new JavaNetReverseProxy2(
new File(new File(System.getProperty("java.io.tmpdir")), "jenkins.io-cache2"));
}
Expand Down
69 changes: 46 additions & 23 deletions src/main/java/org/jvnet/hudson/test/RealJenkinsRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.FileAttribute;
import java.security.MessageDigest;
import java.util.ArrayList;
import java.util.Enumeration;
Expand Down Expand Up @@ -99,6 +100,7 @@
import org.apache.commons.io.FileUtils;
import org.junit.AssumptionViolatedException;
import org.junit.rules.DisableOnDebug;
import org.junit.rules.TemporaryFolder;
import org.junit.rules.TestRule;
import org.junit.rules.Timeout;
import org.junit.runner.Description;
Expand Down Expand Up @@ -193,7 +195,7 @@ public final class RealJenkinsRule implements TestRule {

Process proc;

private File portFile;
private Path portFile;

private Map<String, Level> loggers = new HashMap<>();

Expand Down Expand Up @@ -504,27 +506,31 @@ public static List<String> getJacocoAgentOptions() {
return new Statement() {
@Override public void evaluate() throws Throwable {
System.out.println("=== Starting " + description);
if (war == null) {
war = findJenkinsWar();
}
if (home.get() != null) {
try {
jenkinsOptions("--webroot=" + createTempDirectory("webroot"), "--pluginroot=" + createTempDirectory("pluginroot"));
if (war == null) {
war = findJenkinsWar();
}
if (home.get() != null) {
try {
base.evaluate();
} finally {
stopJenkins();
}
return;
}
try {
home.set(tmp.allocate());
if (!prepareHomeLazily) {
provision();
}
base.evaluate();
} finally {
stopJenkins();
}
return;
}
try {
home.set(tmp.allocate());
if (!prepareHomeLazily) {
provision();
}
base.evaluate();
} finally {
stopJenkins();
try {
deprovision();
tmp.dispose();
} catch (Exception x) {
LOGGER.log(Level.WARNING, null, x);
}
Expand Down Expand Up @@ -652,6 +658,17 @@ public void deprovision() throws Exception {
provisioned = false;
}

/**
* Creates a temporary directory.
* Unlike {@link Files#createTempDirectory(String, FileAttribute...)}
* this will be cleaned up after the test exits (like {@link TemporaryFolder}),
* and will honor {@code java.io.tmpdir} set by Surefire
* after {@code StaticProperty.JAVA_IO_TMPDIR} has been initialized.
*/
public Path createTempDirectory(String prefix) throws IOException {
return tmp.allocate(prefix).toPath();
}

/**
* Returns true if the Jenkins process is alive.
*/
Expand Down Expand Up @@ -752,25 +769,31 @@ public void startJenkins() throws Throwable {
if (prepareHomeLazily && !provisioned) {
provision();
}
var metadata = createTempDirectory("RealJenkinsRule");
var cpFile = metadata.resolve("cp.txt");
String cp = System.getProperty("java.class.path");
Files.writeString(
getHome().toPath().resolve("RealJenkinsRule-cp.txt"),
cpFile,
Stream.of(cp.split(File.pathSeparator)).collect(Collectors.joining(System.lineSeparator())),
StandardCharsets.UTF_8);
List<String> argv = new ArrayList<>(List.of(
new File(javaHome != null ? javaHome : System.getProperty("java.home"), "bin/java").getAbsolutePath(),
"-ea",
"-Dhudson.Main.development=true",
"-DRealJenkinsRule.classpath=" + cpFile,
"-DRealJenkinsRule.location=" + RealJenkinsRule.class.getProtectionDomain().getCodeSource().getLocation(),
"-DRealJenkinsRule.description=" + description,
"-DRealJenkinsRule.token=" + token));
argv.addAll(getJacocoAgentOptions());
for (Map.Entry<String, Level> e : loggers.entrySet()) {
argv.add("-D" + REAL_JENKINS_RULE_LOGGING + e.getKey() + "=" + e.getValue().getName());
}
portFile = File.createTempFile("jenkins-port", ".txt", getHome());
Files.delete(portFile.toPath());
portFile = metadata.resolve("jenkins-port.txt");
argv.add("-Dwinstone.portFileName=" + portFile);
var tmp = System.getProperty("java.io.tmpdir");
if (tmp != null) {
argv.add("-Djava.io.tmpdir=" + tmp);
}
boolean debugging = new DisableOnDebug(null).isDebugging();
if (debugging) {
argv.add("-agentlib:jdwp=transport=dt_socket"
Expand Down Expand Up @@ -821,7 +844,7 @@ public void startJenkins() throws Throwable {
proc = null;
throw new IOException("Jenkins process terminated prematurely with exit code " + exitValue);
}
if (port == 0 && portFile != null && portFile.exists()) {
if (port == 0 && portFile != null && Files.isRegularFile(portFile)) {
port = readPort(portFile);
}
if (port != 0) {
Expand Down Expand Up @@ -904,13 +927,13 @@ private void addTimeout() {
}
}

private static int readPort(File portFile) throws IOException {
String s = Files.readString(portFile.toPath(), StandardCharsets.UTF_8);
private static int readPort(Path portFile) throws IOException {
String s = Files.readString(portFile, StandardCharsets.UTF_8);

// Work around to non-atomic write of port value in Winstone releases prior to 6.1.
// TODO When Winstone 6.2 has been widely adopted, this can be deleted.
if (s.isEmpty()) {
LOGGER.warning(() -> String.format("PortFile: %s exists, but value is still not written", portFile.getAbsolutePath()));
LOGGER.warning(() -> String.format("PortFile: %s exists, but value is still not written", portFile));
return 0;
}

Expand Down Expand Up @@ -1170,7 +1193,7 @@ public static final class Init2 {
public static void run(Object jenkins) throws Exception {
Object pluginManager = jenkins.getClass().getField("pluginManager").get(jenkins);
ClassLoader uberClassLoader = (ClassLoader) pluginManager.getClass().getField("uberClassLoader").get(pluginManager);
ClassLoader tests = new URLClassLoader(Files.readAllLines(Paths.get(System.getenv("JENKINS_HOME"), "RealJenkinsRule-cp.txt"), StandardCharsets.UTF_8).stream().map(Init2::pathToURL).toArray(URL[]::new), uberClassLoader);
ClassLoader tests = new URLClassLoader(Files.readAllLines(Paths.get(System.getProperty("RealJenkinsRule.classpath")), StandardCharsets.UTF_8).stream().map(Init2::pathToURL).toArray(URL[]::new), uberClassLoader);
tests.loadClass("org.jvnet.hudson.test.RealJenkinsRule$Endpoint").getMethod("register").invoke(null);
}

Expand Down
8 changes: 6 additions & 2 deletions src/main/java/org/jvnet/hudson/test/TemporaryDirectoryAllocator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,13 @@ public TemporaryDirectoryAllocator() {
* This directory will be wiped out when {@link TemporaryDirectoryAllocator} gets disposed.
* When this method returns, the directory already exists.
*/
public synchronized File allocate() throws IOException {
public File allocate() throws IOException {
return allocate(withoutSpace ? "jkh" : "j h");
}

synchronized File allocate(String name) throws IOException {
try {
File f = Files.createTempDirectory(base.toPath(), (withoutSpace ? "jkh" : "j h")).toFile();
File f = Files.createTempDirectory(base.toPath(), name).toFile();
tmpDirectories.add(f);
return f;
} catch (IOException e) {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jvnet/hudson/test/UnitTestSupportingPluginManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
public class UnitTestSupportingPluginManager extends PluginManager {

public UnitTestSupportingPluginManager(File rootDir) {
super((ServletContext) null, new File(rootDir, "plugins"));
super((ServletContext) null, rootDir);
}

/** @see LocalPluginManager#loadBundledPlugins */
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jvnet/hudson/test/recipes/WithPluginManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public void setup(JenkinsRule jenkinsRule, WithPluginManager recipe) throws Exce
public void decorateHome(JenkinsRule jenkinsRule, File home) throws Exception {
Class<? extends PluginManager> c = recipe.value();
Constructor<? extends PluginManager> ctr = c.getDeclaredConstructor(File.class);
jenkinsRule.setPluginManager(ctr.newInstance(home));
jenkinsRule.setPluginManager(ctr.newInstance(new File(home, "plugins")));
}
}
}

0 comments on commit 9e8bb87

Please sign in to comment.