db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mi...@apache.org
Subject svn commit: r447736 - in /db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests: harness/ProcessStreamResult.java tests/derbynet/testProperties.java
Date Tue, 19 Sep 2006 05:12:34 GMT
Author: mikem
Date: Mon Sep 18 22:12:33 2006
New Revision: 447736

URL: http://svn.apache.org/viewvc?view=rev&rev=447736
Log:
DERBY-1694
contributed by Sunitha Kambhampati

I was looking at the testProperties issue that caused problems because of test hang. The test
doesnt hang in my environment but I thought I would look to see why the test didnt timeout.

I found couple of issues here:
0. The testProperties.java and several networkserver tests exec new processes to start server,
test properties,shutdown server etc. In some cases, we wait to capture the output from the
subprocess that is started. ProcessStreamResult is used for this purpose. ProcessStreamResult
is part of the harness (see org.apache.derbyTesting.functionTests.harness.ProcessStreamResult)
and it starts a thread to read data from the process's stream and writes it out. Once EOS
(-1) is reached, the thread exits after doing a notifyAll.

1. ProcessStreamResult.Wait() does not work with the timeout case. I think the original intention
of the method that takes the timeout was to force the thread to stop, once the timeout period
has elapsed. The method Wait() does not handle this case.
2. On timeout, the myThread needs to stop its work. The run() method does not handle this
case.
3. testProperties test does not make use of the ProcessStreamResult timeout mechanism.
4. Process's are exec'd in the tests and they are not destroyed within a timeout period. The
network server tests start server using Process, and then cleanup by shutting them down. It
will all work ok, if no deadlock or blocking of process's occur. It seems to me though, that
we should have a way to destroy the processes that are
started as part of each test given a timeout period. Each test must learn to do the cleanup
when it leaves and the test has knowledge of all the sub-processes that it has exec'd. The
current test harness has a class TimedProcess which could work.

In the spirit of incremental development, I am attaching a patch(derby1694.p2.diff.txt and
corresponding stat file - derby1694.p2.stat.txt) that fixes problems 1,2 and 3. I think #4
can be handled as a separate issue/patch.

This patch
-- fixes the timeout handling in ProcessStreamResult. Instance variable 'interrupted' is a
flag to indicate if a timeout has occurred or if the thread's work has been interrupted in
between. The flag 'finished' indicates whether the work has been finished by the thread. Changes
are in Wait() method to make use of wait(timeoutms) if a timeout is specified in ProcessStreamResult.
If timeout time has elapsed, then the interrupted flag is set to true.
-- Adds condition in the run() method to check if interrupted is true. If so, the thread will
stop its work and leave.
-- correctly return if the thread's work was interrupted either because of a timeout or if
it was interrupted.
-- Make use of the ProcessStreamResult with a timeout setting of 2 minutes in testProperties
test. Note, the timeout handling only comes into effect when ProcessStreamResult.Wait() method
is called.

other notes:
-- when you run a test, the suite property for timeout does not get picked up. I think this
is intentional behavior.
-- Issues mentioned above are not specific to just testProperties but exist for other networkserver
tests. There are a total of 7 files in derbynet that make use of this.


Modified:
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/harness/ProcessStreamResult.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/testProperties.java

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/harness/ProcessStreamResult.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/harness/ProcessStreamResult.java?view=diff&rev=447736&r1=447735&r2=447736
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/harness/ProcessStreamResult.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/harness/ProcessStreamResult.java
Mon Sep 18 22:12:33 2006
@@ -32,11 +32,30 @@
 	protected OutputStreamWriter outStream;
 	// Encoding to be used to read output of test jvm process
 	protected String encoding;
+
+    /**
+     * Flag to find out if the work was finished 
+     * successfully without being interrupted 
+     * in between because of a timeout setting
+     */
 	protected boolean finished;
 	protected IOException ioe;
 	protected Thread myThread;
 	protected long startTime;
+    
+    /**
+     * Flag to keep state of whether the myThread has timed out.
+     * When interrupted is true, the myThread will exit 
+     * from its work. 
+     */
 	protected boolean interrupted;
+    
+    /**
+     * time in minutes for myThread to timeout in case it 
+     * has not finished its work before that.
+     * timeout handling only comes into effect only when Wait()
+     * is called.
+     */
 	protected int timeout;
 
 	public ProcessStreamResult(InputStream in, BufferedOutputStream bos,
@@ -92,7 +111,10 @@
         	else
         		inStream = new InputStreamReader(in);
 			
-			while ((valid = inStream.read(ca, 0, ca.length)) != -1)
+            // keep reading from the stream as long as we have not 
+            // timed out
+			while (((valid = inStream.read(ca, 0, ca.length)) != -1) &&
+                    !interrupted)
 			{
 			    //System.out.println("Still reading thread: " + tname);
 /*				if (timeout > 0) {
@@ -124,17 +146,51 @@
 			//ioe.printStackTrace();
 		}
 
+        // if we timed out, then just leave
+        if ( interrupted )
+            return;
+        
 		synchronized (this)
 		{
+            // successfully finished the work, notifyAll and leave.
 			finished = true;
 			notifyAll();
 		}
 	}
 
+    /**
+     * Wait till the myThread has finished its work or incase a timeout was set on this 
+     * object, then to set a flag to indicate the myThread to leave at the end of the 
+     * timeout period.
+     * 
+     * Behavior is as follows:
+     * 1) If timeout is set to a valid value (>0) - in this case, if myThread has not
+     * finished its work by the time this method was called, then it will wait
+     * till the timeout has elapsed or if the myThread has finished its work.
+     * 
+     * 2)If timeout is not set ( <= 0) - in this case, if myThread has not
+     * finished its work by the time this method was called, then it will wait
+     * till myThread has finished its work.
+     * 
+     * If timeout is set to a valid value, and the timeout amount of time has elapsed, 
+     * then the interrupted  flag is set to true to indicate that it is time for the 
+     * myThread to stop its work and leave.
+     *
+     * @return true if the timeout happened before myThread work was finished
+     *         else false
+     * @throws IOException
+     */
 	public boolean Wait() throws IOException
 	{
 	    synchronized(this)
 	    {
+            // It is possible that we have finished the work 
+            // by the time this method Wait() was called,
+            // so need to check if that is the case, before we
+            // go into a wait.
+            if ( finished )
+                return interrupted;
+            
 			if (timeout > 0) {
 				long millis = System.currentTimeMillis();
 
@@ -144,18 +200,32 @@
 
 				if (mins > timeout)
 				{
+                    interrupted = true;
 					return interrupted;
 				}
 			}
 			try
 			{
-				while (!finished && !interrupted)
-				{
-					wait();
-				}
-			}
+                // find timeout in milliseconds
+                long timeoutms = timeout * 60 *1000L;
+                
+                if ( timeout > 0 )
+                    // wait till notified or till timeoutms has elapsed
+                    wait(timeoutms);
+                else
+                    wait(); // wait till notified
+                
+                // if myThread didnt finish its work and we reached
+                // here, that means we just timedout. 
+                // In that case, indicate that we were interrupted and leave.
+                // myThread will read the value of interrupted and 
+                // stop its work and leave.
+    		    if ( !finished )
+                    interrupted = true;
+            }
 			catch (InterruptedException ie)
 			{
+                interrupted = true;
 				System.out.println("Interrupted: " + ie.toString());
 			}
 	    }

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/testProperties.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/testProperties.java?view=diff&rev=447736&r1=447735&r2=447736
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/testProperties.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/testProperties.java
Mon Sep 18 22:12:33 2006
@@ -56,6 +56,15 @@
 	private static jvm jvm;
 	private static Vector vCmd;
     private static  BufferedOutputStream bos = null;
+    
+    /**
+     * For each new exec process done as part of this test, set 
+     * timeout for ProcessStreamResult after which the thread that 
+     * handles the streams for the process exits.  Timeout is in minutes. 
+     * Note: timeout handling will only come into effect when 
+     * ProcessStreamResult#Wait() is called
+     */
+    private static String timeoutMinutes = "2";
 
     //Command to start server specifying system properties without values
     private static String[] startServerCmd =
@@ -115,8 +124,8 @@
 	 * Execute the given command and optionally wait and dump the results to standard out
 	 *
 	 * @param args	command and arguments
-	 * @param wait  true =wait for completion and dump output, false don't wait and
-     * ignore the output.
+	 * @param wait  true =wait for either completion or timeout time and dump output, 
+     * false don't wait and ignore the output.
 	 * @exception Exception
 	 */
 
@@ -138,13 +147,16 @@
         }
 		// Start a process to run the command
 		Process pr = execCmd(args);
-        prout = new ProcessStreamResult(pr.getInputStream(), _bos, null);
-        prerr = new ProcessStreamResult(pr.getErrorStream(), _bos, null);
+        
+        // Note, the timeout handling will only come into effect when we make
+        // the Wait() call on ProcessStreamResult. 
+        prout = new ProcessStreamResult(pr.getInputStream(), _bos, timeoutMinutes);
+        prerr = new ProcessStreamResult(pr.getErrorStream(), _bos, timeoutMinutes);
         
         if (!wait)
             return;
 
-		// wait until all the results have been processed
+		// wait until all the results have been processed or if we timed out
 		prout.Wait();
 		prerr.Wait();
         _bos.flush();



Mime
View raw message