Updated Branches: refs/heads/master 35ab598d1 -> f081092b8
ProcessUtil cleanup - possible resource leak closed - file content read uses now commons-lang FileUtils - Added unit tests Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/f081092b Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/f081092b Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/f081092b Branch: refs/heads/master Commit: f081092b8051fd78cd4dd924f7193d86c4920a0f Parents: 35ab598 Author: Laszlo Hornyak <[email protected]> Authored: Tue Jun 18 22:09:05 2013 +0200 Committer: frank <[email protected]> Committed: Mon Aug 12 14:50:09 2013 -0700 ---------------------------------------------------------------------- utils/src/com/cloud/utils/ProcessUtil.java | 20 +++---- utils/test/com/cloud/utils/ProcessUtilTest.java | 57 ++++++++++++++++++++ 2 files changed, 63 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f081092b/utils/src/com/cloud/utils/ProcessUtil.java ---------------------------------------------------------------------- diff --git a/utils/src/com/cloud/utils/ProcessUtil.java b/utils/src/com/cloud/utils/ProcessUtil.java index c9fdf35..7f16f98 100644 --- a/utils/src/com/cloud/utils/ProcessUtil.java +++ b/utils/src/com/cloud/utils/ProcessUtil.java @@ -16,24 +16,20 @@ // under the License. package com.cloud.utils; -import java.io.BufferedReader; import java.io.File; import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.IOException; -import java.io.InputStreamReader; +import java.util.Properties; import javax.naming.ConfigurationException; +import org.apache.commons.io.FileUtils; import org.apache.log4j.Logger; -import com.cloud.utils.PropertiesUtil; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.OutputInterpreter; import com.cloud.utils.script.Script; -import java.util.Properties; - public class ProcessUtil { private static final Logger s_logger = Logger.getLogger(ProcessUtil.class.getName()); @@ -67,11 +63,9 @@ public class ProcessUtil { if (!pidFile.exists()) { throw new ConfigurationException("Unable to write to " + pidFile.getAbsolutePath() + ". Are you sure you're running as root?"); } - - final FileInputStream is = new FileInputStream(pidFile); - final BufferedReader reader = new BufferedReader(new InputStreamReader(is)); - final String pidLine = reader.readLine(); - if (pidLine == null) { + + final String pidLine = FileUtils.readFileToString(pidFile).trim(); + if (pidLine.isEmpty()) { throw new ConfigurationException("Java process is being started twice. If this is not true, remove " + pidFile.getAbsolutePath()); } try { @@ -101,9 +95,7 @@ public class ProcessUtil { final String pid = parser.getLine(); - final FileOutputStream strm = new FileOutputStream(pidFile); - strm.write((pid + "\n").getBytes()); - strm.close(); + FileUtils.writeStringToFile(pidFile, pid + "\n"); } catch (final IOException e) { throw new CloudRuntimeException("Unable to create the " + pidFile.getAbsolutePath() + ". Are you running as root?", e); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f081092b/utils/test/com/cloud/utils/ProcessUtilTest.java ---------------------------------------------------------------------- diff --git a/utils/test/com/cloud/utils/ProcessUtilTest.java b/utils/test/com/cloud/utils/ProcessUtilTest.java new file mode 100644 index 0000000..266ae1a --- /dev/null +++ b/utils/test/com/cloud/utils/ProcessUtilTest.java @@ -0,0 +1,57 @@ +package com.cloud.utils; + +import java.io.File; +import java.io.IOException; + +import javax.naming.ConfigurationException; + +import junit.framework.Assert; + +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang.SystemUtils; +import org.junit.After; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Test; + +public class ProcessUtilTest { + + File pidFile; + + @Before + public void setup() throws IOException { + pidFile = File.createTempFile("test", ".pid"); + } + + @After + public void cleanup() { + FileUtils.deleteQuietly(pidFile); + } + + @Test + public void pidCheck() throws ConfigurationException, IOException { + Assume.assumeTrue(SystemUtils.IS_OS_LINUX); + FileUtils.writeStringToFile(pidFile, "123456\n"); + ProcessUtil.pidCheck(pidFile.getParent(), pidFile.getName()); + String pidStr = FileUtils.readFileToString(pidFile); + Assert.assertFalse("pid can not be blank", pidStr.isEmpty()); + int pid = Integer.parseInt(pidStr.trim()); + int maxPid = Integer.parseInt(FileUtils.readFileToString(new File("/proc/sys/kernel/pid_max")).trim()); + Assert.assertTrue(pid <= maxPid); + } + + @Test(expected = ConfigurationException.class) + public void pidCheckBadPid() throws ConfigurationException, IOException { + Assume.assumeTrue(SystemUtils.IS_OS_LINUX); + FileUtils.writeStringToFile(pidFile, "intentionally not number"); + ProcessUtil.pidCheck(pidFile.getParent(), pidFile.getName()); + } + + @Test(expected = ConfigurationException.class) + public void pidCheckEmptyPid() throws ConfigurationException, IOException { + Assume.assumeTrue(SystemUtils.IS_OS_LINUX); + FileUtils.writeStringToFile(pidFile, "intentionally not number"); + ProcessUtil.pidCheck(pidFile.getParent(), pidFile.getName()); + } + +}
