Commit 5558171b authored by Neil Fuller's avatar Neil Fuller
Browse files

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
Change-Id: I92cc8b150378bb41f49f996422e1a7bc19801b67
parent 03aae052
......@@ -84,12 +84,6 @@ public class Support_TestWebServer implements Support_HttpConstants {
/* 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() {
}
......@@ -165,14 +159,6 @@ public class Support_TestWebServer implements Support_HttpConstants {
this.maxChunkSize = maxChunkSize;
}
/**
* 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
......@@ -195,10 +181,6 @@ public class Support_TestWebServer implements Support_HttpConstants {
return pathToRequest;
}
public int getNumAcceptedConnections() {
return acceptedConnections;
}
/**
* Cause the thread accepting connections on the server socket to close
*/
......@@ -217,12 +199,8 @@ public class Support_TestWebServer implements Support_HttpConstants {
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 @@ public class Support_TestWebServer implements Support_HttpConstants {
/**
* 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 @@ public class Support_TestWebServer implements Support_HttpConstants {
// 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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment