Remove a race hazard from the execrable Support_TestWebServer
Use of this class was resulting in threads being left in a
spinning state due to the accept socket being closed but
"running" still being true: the exception thrown from
Socket.accept() was being swallowed.
This is likely because there is a race between the thread
actually starting (i.e. run() actually executing)
and, in short-lived tests, the server being shutdown:
if AcceptThread.close() was called before AcceptThread.run()
then when run() actually executed it would loop forever.
Some dead code has been removed.
Test: Ran the CTS tests
Bug: 29820565
Bug: 29365528
(cherry picked from commit 5558171bce9b70743bed9f6d3543d2eb15d51bb0)
Change-Id: I895ecbd171c9495ddda85de740519ba1b2c51d62
diff --git a/support/src/test/java/tests/support/Support_TestWebServer.java b/support/src/test/java/tests/support/Support_TestWebServer.java
index 33216a7..4ccf92c 100644
--- a/support/src/test/java/tests/support/Support_TestWebServer.java
+++ b/support/src/test/java/tests/support/Support_TestWebServer.java
@@ -84,12 +84,6 @@
/* If set, this indicates the reason for redirection */
int redirectCode = -1;
- /* Set the number of connections the server will accept before shutdown */
- int acceptLimit = 100;
-
- /* Count of number of accepted connections */
- int acceptedConnections = 0;
-
public Support_TestWebServer() {
}
@@ -166,14 +160,6 @@
}
/**
- * Call this to specify the maximum number of sockets to accept
- * @param limit The number of sockets to accept
- */
- public void setAcceptLimit(int limit) {
- acceptLimit = limit;
- }
-
- /**
* Call this to indicate redirection port requirement.
* When this value is set, the server will respond to a request with
* a redirect code with the Location response header set to the value
@@ -195,10 +181,6 @@
return pathToRequest;
}
- public int getNumAcceptedConnections() {
- return acceptedConnections;
- }
-
/**
* Cause the thread accepting connections on the server socket to close
*/
@@ -217,12 +199,8 @@
class AcceptThread extends Thread {
ServerSocket ss = null;
- boolean running = false;
+ volatile boolean closed = false;
- /**
- * @param port the port to use, or 0 to let the OS choose.
- * Hard-coding ports is evil, so always pass 0!
- */
public int init() throws IOException {
ss = new ServerSocket(0);
ss.setSoTimeout(5000);
@@ -233,15 +211,10 @@
/**
* Main thread responding to new connections
*/
- public synchronized void run() {
- running = true;
- while (running) {
+ public void run() {
+ while (!closed) {
try {
Socket s = ss.accept();
- acceptedConnections++;
- if (acceptedConnections >= acceptLimit) {
- running = false;
- }
new Thread(new Worker(s), "additional worker").start();
} catch (SocketException e) {
log(e.getMessage());
@@ -255,7 +228,7 @@
// Close this socket
public void close() {
try {
- running = false;
+ closed = true;
/* Stop server socket from processing further. Currently
this does not cause the SocketException from ss.accept
therefore the acceptLimit functionality has been added
