From 85a7efa09c2487d7a1907e4f70115fb7f579eca1 Mon Sep 17 00:00:00 2001
From: Pyry Haulos <phaulos@google.com>
Date: Fri, 26 Sep 2014 17:29:34 -0700
Subject: [PATCH] Make TestLogParser and DeqpInstrumentation more robust:

 * Fix potential IndexOutOfBounds in TestLogParser
 * Fix bug in log file polling logic
 * Don't stop parsing remaining log data immediately when test process
   finishes

Change-Id: Iae8d153c6d0a32f9574d6d7131f3834479b83cb1
---
 .../deqp/testercore/DeqpInstrumentation.java       | 85 ++++++++++++++--------
 .../deqp/testercore/TestLogParser.java             | 20 ++---
 2 files changed, 63 insertions(+), 42 deletions(-)

diff --git a/android/package/src/com/drawelements/deqp/testercore/DeqpInstrumentation.java b/android/package/src/com/drawelements/deqp/testercore/DeqpInstrumentation.java
index 5c45949..3c0afb9 100644
--- a/android/package/src/com/drawelements/deqp/testercore/DeqpInstrumentation.java
+++ b/android/package/src/com/drawelements/deqp/testercore/DeqpInstrumentation.java
@@ -43,8 +43,10 @@ import java.io.IOException;
 
 public class DeqpInstrumentation extends Instrumentation
 {
-	private static final String	LOG_TAG		= "dEQP/Instrumentation";
-	private static final long	TIMEOUT_MS	= 40000; // 40s Timeouts if no log is produced.
+	private static final String	LOG_TAG					= "dEQP/Instrumentation";
+	private static final long	LAUNCH_TIMEOUT_MS		= 10000;
+	private static final long	NO_DATA_TIMEOUT_MS		= 1000;
+	private static final long	NO_ACTIVITY_SLEEP_MS	= 100;
 
 	private String				m_cmdLine;
 	private String				m_logFileName;
@@ -79,61 +81,84 @@ public class DeqpInstrumentation extends Instrumentation
 	public void onStart () {
 		super.onStart();
 
+		final RemoteAPI		remoteApi	= new RemoteAPI(getTargetContext(), m_logFileName);
+		final TestLogParser	parser		= new TestLogParser();
+
 		try
 		{
 			Log.d(LOG_TAG, "onStart");
 
-			String			testerName		= "";
-			RemoteAPI		remoteApi		= new RemoteAPI(getTargetContext(), m_logFileName);
-			TestLogParser	parser			= null;
-			long			lastMoreLogMs	= 0;
+			final String	testerName		= "";
+			final File		logFile			= new File(m_logFileName);
 
-			{
-				File log = new File(m_logFileName);
-				if (log.exists())
-					log.delete();
-			}
+			if (logFile.exists())
+				logFile.delete(); // Remove log file left by previous session
 
 			remoteApi.start(testerName, m_cmdLine, null);
 
-			// Wait for execution to start
-			Thread.sleep(1000); // 1s
-
-			parser = new TestLogParser();
-			parser.init(this, m_logFileName, m_logData);
-
-			lastMoreLogMs = System.currentTimeMillis();
-
-			while (remoteApi.isRunning())
 			{
-				if (parser.parse())
-					lastMoreLogMs = System.currentTimeMillis();
-				else
+				final long startTimeMs = System.currentTimeMillis();
+
+				while (true)
 				{
-					long currentTimeMs = System.currentTimeMillis();
+					final long timeSinceStartMs = System.currentTimeMillis()-startTimeMs;
 
-					if (currentTimeMs - lastMoreLogMs >= TIMEOUT_MS)
+					if (logFile.exists())
+						break;
+					else if (timeSinceStartMs > LAUNCH_TIMEOUT_MS)
 					{
 						remoteApi.kill();
-						break;
+						throw new Exception("Timeout while waiting for log file");
 					}
-
-					Thread.sleep(100); // Wait 100ms
+					else
+						Thread.sleep(NO_ACTIVITY_SLEEP_MS);
 				}
 			}
 
-			parser.parse();
-			parser.deinit();
+			parser.init(this, m_logFileName, m_logData);
+
+			{
+				long lastDataMs = System.currentTimeMillis();
+
+				while (true)
+				{
+					if (parser.parse())
+						lastDataMs = System.currentTimeMillis();
+					else if (!remoteApi.isRunning())
+					{
+						final long timeSinceLastDataMs = System.currentTimeMillis()-lastDataMs;
+
+						if (timeSinceLastDataMs > NO_DATA_TIMEOUT_MS)
+							break; // Assume no data is available for reading any more
+					}
+					else
+						Thread.sleep(NO_ACTIVITY_SLEEP_MS);
+				}
+			}
 
 			finish(0, new Bundle());
 		}
 		catch (Exception e)
 		{
+			Log.e(LOG_TAG, "Exception", e);
+
 			Bundle info = new Bundle();
 
 			info.putString("Exception", e.getMessage());
 			finish(1, new Bundle());
 		}
+		finally
+		{
+			try
+			{
+				parser.deinit();
+			}
+			catch (Exception e)
+			{
+				Log.w(LOG_TAG, "Got exception while closing log", e);
+			}
+			remoteApi.kill();
+		}
 	}
 
 	public void testCaseResult (String code, String details)
diff --git a/android/package/src/com/drawelements/deqp/testercore/TestLogParser.java b/android/package/src/com/drawelements/deqp/testercore/TestLogParser.java
index 1945463..f5befc8 100644
--- a/android/package/src/com/drawelements/deqp/testercore/TestLogParser.java
+++ b/android/package/src/com/drawelements/deqp/testercore/TestLogParser.java
@@ -32,12 +32,7 @@ import java.io.IOException;
 public class TestLogParser
 {
 	static {
-		if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.GINGERBREAD) {
-			System.loadLibrary("testercore");
-		} else {
-			// Pre-gingerbread version without EGL tests.
-			System.loadLibrary("testercore_legacy");
-		}
+		System.loadLibrary("testercore");
 	}
 
 	private long				m_nativePointer;
@@ -94,21 +89,22 @@ public class TestLogParser
 		assert m_log != null;
 
 		boolean	gotData	= false;
-		int		size	= 0;
 
 		while (true)
 		{
-			if (m_log.available() == 0)
+			final int numAvailable = m_log.available();
+
+			if (numAvailable <= 0)
 				break;
 
-			size = m_log.read(m_buffer, 0, m_log.available());
+			final int numRead = m_log.read(m_buffer, 0, Math.min(numAvailable, m_buffer.length));
 
-			assert size != -1;
+			assert numRead > 0;
 
-			m_logRead += size;
+			m_logRead += numRead;
 
 			gotData = true;
-			nativeParse(m_nativePointer, m_instrumentation, m_buffer, size);
+			nativeParse(m_nativePointer, m_instrumentation, m_buffer, numRead);
 		}
 
 		return gotData;
-- 
2.7.4