This is the mail archive of the
mauve-patches@sourceware.org
mailing list for the Mauve project.
Re: RFC: Harness with no busy-waiting
On Tue, 2006-06-27 at 10:41 +0200, Mark Wielaard wrote:
> Hi Anthony,
>
> On Mon, 2006-06-26 at 15:27 -0400, Anthony Balkissoon wrote:
> > This removes the busy waiting from the Harness and replaces it with
> > blocking read() calls using Sockets with specified timeouts.
> >
> > This is RFC because I'd like it to be tested to see if it speeds things
> > up as much as Mark's workaround before it is checked in.
>
> I don't really like this design. It puts even more stuff inside the
> RunnerProcess which really should be as simple as possible so you can
> easily run and debug it. Otherwise you might be trying to debug the
> infrastructure and not the test itself. And by adding Sockets you a also
> make it very hard/impossible to run single tests without the Harness to
> really debug them. Please just use normal streams for communication
> between the RunnerProcess and whatever calls it. And add a old-fashion
> timeout with Object.wait()/notify() to the Harness, or if you want
> something more fancy there add a nio.channel.Selector. But please don't
> make the RunnerProcess depend on too much infrastructure.
>
> Cheers,
>
> Mark
>
Okay, here's a second attempt, no Sockets, no tricks. Still RFC.
Comments?
2006-06-27 Anthony Balkissoon <abalkiss@redhat.com>
* Harness.java:
(bcp_timeout): Removed this field.
(testIsHung): Likewise.
(runner_lock): Likewise.
(getClasspathInstallString): Return an empty string instead of null
when no bootclasspath is detected.
(getBootClassPath): Removed the TimeoutWatcher and replaced the busy
waiting with a blocking readLine().
(runTest): Redesigned to replace busy waiting with blocking readLine()
call.
(TimeoutWatcher.run): If the timeout occurs, kill the RunnerProcess and
restart it instead of notifying the Harness and letting the Harness do
the work.
(DetectBootclasspath.main): If the bootclasspath isn't detectable,
return a String saying so.
* RunnerProcess.java:
(FAILED_TO_LOAD_DESCRIPTION): New field.
(UNCAUGHT_EXCEPTION_DESCRIPTION): Likewise.
(main): If the testname is null, call System.exit.
(runtest): Set the description earlier to avoid NPE. Handle loading
exceptions and uncaught exceptions better.
(runAndReport): Handle loading exceptions and uncaught exceptions.
--Tony
Index: Harness.java
===================================================================
RCS file: /cvs/mauve/mauve/Harness.java,v
retrieving revision 1.15
diff -u -r1.15 Harness.java
--- Harness.java 24 Jun 2006 17:33:22 -0000 1.15
+++ Harness.java 27 Jun 2006 18:50:08 -0000
@@ -79,9 +79,6 @@
// The location of the eclipse-ecj.jar file
private static String ecjJarLocation = null;
- // How long the bootclasspath finder program can run before termination
- private static long bcp_timeout = 60000;
-
// How long a test may run before it is considered hung
private static long runner_timeout = 60000;
@@ -138,12 +135,6 @@
// A watcher to determine if runnerProcess is hung
private static TimeoutWatcher runner_watcher = null;
- // A flag indicating whether or not runnerProcess is hung
- private static boolean testIsHung = false;
-
- // A lock used for synchronizing access to testIsHung
- private static Object runner_lock = new Object();
-
// The arguments used when this Harness was invoked, we use this to create an
// appropriate RunnerProcess
private static String[] harnessArgs = null;
@@ -447,7 +438,7 @@
// " -bootclasspath " followed by the detected path.
if (temp != null)
return " -bootclasspath " + temp;
- return temp;
+ return "";
}
// This section is for bootclasspath's specified with
@@ -484,52 +475,22 @@
new BufferedReader
(new InputStreamReader(p.getInputStream()));
String bcpOutput = null;
- // Create a timer to watch this new process.
- TimeoutWatcher tw = new TimeoutWatcher(bcp_timeout);
- tw.start();
while (true)
{
- // If for some reason the process hangs, return null, indicating we
- // cannot auto-detect the bootclasspath.
- if (testIsHung)
+ // This readLine() is a blocking call. This will hang if the
+ // bootclasspath finder hangs.
+ bcpOutput = br.readLine();
+ if (bcpOutput.equals("BCP_FINDER:can't_find_bcp_"))
{
- synchronized (runner_lock)
- {
- testIsHung = false;
- }
- br.close();
- p.destroy();
+ // This means the auto-detection failed.
return null;
}
- if (br.ready())
+ else if (bcpOutput.startsWith("BCP_FINDER:"))
{
- bcpOutput = br.readLine();
- if (bcpOutput == null)
- {
- // This means the auto-detection failed.
- tw.stop();
- return null;
- }
- if (bcpOutput.startsWith("BCP_FINDER:"))
- {
- tw.stop();
- return bcpOutput.substring(11);
- }
- else
- System.out.println(bcpOutput);
+ return bcpOutput.substring(11);
}
- else
- {
- // FIXME - Quickworkaround for busy-waiting, needs proper FIX.
- try
- {
- Thread.sleep(100);
- }
- catch (InterruptedException ie)
- {
- // ignore
- }
- }
+ else
+ System.out.println(bcpOutput);
}
}
catch (IOException ioe)
@@ -774,9 +735,8 @@
{
String tn = stripPrefix(testName.replace(File.separatorChar, '.'));
String outputFromTest;
- StringBuffer sb = new StringBuffer();
boolean invalidTest = false;
- int temp = -1;
+ int temp = -99;
// Start the timeout watcher
if (runner_watcher.isAlive())
@@ -789,113 +749,71 @@
while (true)
{
- // This while loop polls for output from the test process and
- // passes it to System.out unless it is the signal that the
- // test finished properly. Also checks to see if the watcher
- // thread has declared the test hung and if so ends the process.
- if (testIsHung)
- {
- System.err.print(sb.toString());
- if (testName.equals("_confirm_startup_"))
- {
- System.err.println("ERROR: Cannot create test runner process. Exit");
- System.exit(1);
- }
- synchronized (runner_lock)
- {
- testIsHung = false;
- }
- try
- {
- runner_in.close();
- runner_in_err.close();
- runner_out.close();
- runnerProcess.destroy();
- }
- catch (IOException e)
- {
- System.err.println("Could not close the interprocess pipes.");
- System.exit(- 1);
- }
- initProcess(harnessArgs);
- break;
- }
+ // Continue getting output from the RunnerProcess until it
+ // signals the test completed or was invalid, or until the
+ // TimeoutWatcher stops the RunnerProcess forcefully.
try
{
- if (runner_in_err.ready())
- sb.append(runner_in_err.readLine() + "\n");
- if (runner_in.ready())
+ outputFromTest = runner_in.readLine();
+ if (outputFromTest == null)
{
- outputFromTest = runner_in.readLine();
- if (outputFromTest.startsWith("RunnerProcess:"))
- {
- invalidTest = false;
- // This means the RunnerProcess has sent us back some
- // information. This could be telling us that a check() call
- // was made and we should reset the timer, or that the
- // test passed, or failed, or that it wasn't a test.
- if (outputFromTest.endsWith("restart-timer"))
- runner_watcher.reset();
- else if (outputFromTest.endsWith("pass"))
- {
- temp = 0;
- break;
- }
- else if (outputFromTest.indexOf("fail-") != -1)
- {
- total_check_fails +=
- Integer.parseInt(outputFromTest.substring(19));
- temp = 1;
- break;
- }
- else if (outputFromTest.endsWith("not-a-test"))
- {
- // Temporarily decrease the total number of tests,
- // because it will be incremented later even
- // though the test was not a real test.
- invalidTest = true;
- total_tests--;
- temp = 0;
- break;
- }
- }
- else if (outputFromTest.equals("_startup_okay_") ||
- outputFromTest.equals("_data_dump_okay_"))
- return;
- else
- // This means it was just output from the test, like a
- // System.out.println within the test itself, we should
- // pass these on to stdout.
- System.out.println(outputFromTest);
+ // This means the test hung.
+ temp = - 1;
+ break;
}
- else
+ else if (outputFromTest.startsWith("RunnerProcess:"))
{
- if (sb.length() != 0)
- {
- System.err.print(sb.toString());
- sb = new StringBuffer();
- }
-
- // FIXME - Quickworkaround for busy-waiting, needs proper FIX.
- try
+ invalidTest = false;
+ // This means the RunnerProcess has sent us back some
+ // information. This could be telling us that a check() call
+ // was made and we should reset the timer, or that the
+ // test passed, or failed, or that it wasn't a test.
+ if (outputFromTest.endsWith("restart-timer"))
+ runner_watcher.reset();
+ else if (outputFromTest.endsWith("pass"))
+ {
+ temp = 0;
+ break;
+ }
+ else if (outputFromTest.indexOf("fail-loading") != -1)
+ {
+ temp = 1;
+ System.out.println(outputFromTest.substring(27));
+ }
+ else if (outputFromTest.indexOf("fail-") != - 1)
{
- Thread.sleep(100);
+ total_check_fails += Integer.parseInt(outputFromTest.substring(19));
+ temp = 1;
+ break;
}
- catch (InterruptedException ie)
+ else if (outputFromTest.endsWith("not-a-test"))
{
- // ignore
+ // Temporarily decrease the total number of tests,
+ // because it will be incremented later even
+ // though the test was not a real test.
+ invalidTest = true;
+ total_tests--;
+ temp = 0;
+ break;
}
}
+ else if (outputFromTest.equals("_startup_okay_")
+ || outputFromTest.equals("_data_dump_okay_"))
+ return;
+ else
+ // This means it was just output from the test, like a
+ // System.out.println within the test itself, we should
+ // pass these on to stdout.
+ System.out.println(outputFromTest);
}
catch (IOException e)
{
}
}
- System.err.print(sb.toString());
if (temp == -1)
{
- // This means the watcher thread had to stop the process
- // from running. So this is a fail.
+ // This means the watcher thread had to stop the process
+ // from running. So this is a fail.
if (verbose)
System.out.println(" FAIL: timed out. \nTEST FAILED: timeout " +
tn);
@@ -1224,12 +1142,20 @@
}
if (shouldContinue)
{
- // The test is hung, set testIsHung to true so the process will be
- // destroyed and restarted.
- synchronized (runner_lock)
+ // The test is hung, destroy and restart the RunnerProcess.
+ try
+ {
+ runnerProcess.destroy();
+ runner_in.close();
+ runner_in_err.close();
+ runner_out.close();
+ }
+ catch (IOException e)
{
- testIsHung = true;
+ System.err.println("Could not close the interprocess pipes.");
+ System.exit(- 1);
}
+ initProcess(harnessArgs);
}
}
}
@@ -1279,7 +1205,7 @@
System.err.println("WARNING: Cannot auto-detect the " +
"bootclasspath for your VM, please file a bug report" +
" specifying which VM you are testing.");
- return;
+ temp = "can't_find_bcp_";
}
}
System.out.println(result + temp);
Index: RunnerProcess.java
===================================================================
RCS file: /cvs/mauve/mauve/RunnerProcess.java,v
retrieving revision 1.9
diff -u -r1.9 RunnerProcess.java
--- RunnerProcess.java 27 Jun 2006 15:14:06 -0000 1.9
+++ RunnerProcess.java 27 Jun 2006 18:50:08 -0000
@@ -51,6 +51,12 @@
// A description of files that are not tests
private static final String NOT_A_TEST_DESCRIPTION = "not-a-test";
+ // A description of files that fail to load
+ private static final String FAIL_TO_LOAD_DESCRIPTION = "failed-to-load";
+
+ // A description of a test that throws an uncaught exception
+ private static final String UNCAUGHT_EXCEPTION_DESCRIPTION = "uncaught-exception";
+
// Total number of harness.check calls since the last checkpoint
private int count = 0;
@@ -195,8 +201,10 @@
try
{
testname = in.readLine();
+ if (testname == null)
+ System.exit(0);
if (testname.equals("_dump_data_"))
- {
+ {
if (useEMMA)
dumpCoverageData();
else
@@ -238,6 +246,7 @@
System.runFinalization();
currentResult = new TestResult(name.substring(12));
+ description = name;
checkPoint(null);
@@ -256,6 +265,7 @@
}
catch (Throwable ex)
{
+ description = FAIL_TO_LOAD_DESCRIPTION;
// Maybe the file was marked not-a-test, check that before we report
// it as an error
try
@@ -282,28 +292,35 @@
{
}
- String d = "FAIL: " + stripPrefix(name)
- + "uncaught exception when loading";
- currentResult.addException(ex, "failed loading class ",
- "couldn't load: " + name);
- if (verbose || exceptions)
- d += ": " + ex.toString();
-
- if (exceptions)
- ex.printStackTrace(System.out);
+ String r = getStackTraceString(ex, " ");
+ currentResult.addException(ex, "failed loading class ", r);
+
debug(ex);
if (ex instanceof InstantiationException
|| ex instanceof IllegalAccessException)
debug("Hint: is the code we just loaded a public non-abstract "
+ "class with a public nullary constructor???");
- ++failures;
- ++total;
+
+
+ if (!verbose)
+ System.out.println ("FAIL: " + stripPrefix(name) + ": exception when loading:");
+ else
+ {
+ System.out.println ("TEST: "+stripPrefix(name));
+ System.out.println(" FAIL: exception when loading");
+ }
+ if (exceptions)
+ System.out.println(getStackTraceString(ex, " "));
+
+ if (verbose)
+ System.out.println("TEST FAILED: exception when loading "
+ + stripPrefix(name));
+ return;
}
// If the harness started okay, now we run the test.
if (t != null)
{
- description = name;
try
{
if (verbose)
@@ -313,18 +330,24 @@
}
catch (Throwable ex)
{
- if (failures == 0 && !verbose)
- System.out.println ("FAIL: " + stripPrefix(name) + ":");
- removeSecurityManager();
+
String d = exceptionDetails(ex, name, exceptions);
String r = getStackTraceString(ex, " ");
- currentResult.addException(ex, d, r);
+
+ if (failures == 0 && !verbose)
+ System.out.println ("FAIL: " + stripPrefix(name) + ":");
System.out.println(d);
+ removeSecurityManager();
+ currentResult.addException(ex, d, r);
+
if (exceptions)
System.out.println(getStackTraceString(ex, " "));
debug(ex);
- ++failures;
- ++total;
+
+ if (verbose)
+ System.out.println("TEST FAILED: uncaught exception "
+ + stripPrefix(description));
+ description = UNCAUGHT_EXCEPTION_DESCRIPTION;
}
}
if (report != null)
@@ -365,6 +388,17 @@
System.out.println("RunnerProcess:not-a-test");
return;
}
+ else if (harness.description.equals(FAIL_TO_LOAD_DESCRIPTION))
+ {
+ System.out.println("RunnerProcess:fail-0");
+ return;
+ }
+ else if (harness.description.equals(UNCAUGHT_EXCEPTION_DESCRIPTION))
+ {
+ System.out.println("RunnerProcess:fail-0");
+ return;
+ }
+
// Print out a summary.
int temp = harness.done();
// Print the report if necessary.