Test reports success and then hangs

[sucaba]

What: Script test hangs after successful completion, reporting results and browser closing
Where: ScriptSharp.Testing version=0.7.5 + NUnit 2.6.1
When Started: After implementation of the test web server switched from the Cassini to the "quick and dirty" implementation

Details: Hang happens after return from the WebTest.StopWebServer(). Probably during stopping server thread unhandled exception caused unexpected behavior.

Test Output (with debugger):
"""
...
Completed; failures = 0; total = 111

---- UNHANDLED EXCEPTION ----
Thread Name:
System.IO.IOException: Unable to write data to the transport connection: An existing connection was forcibly closed by the remote host. ---> System.Net.Sockets.SocketException: An existing connection was forcibly closed by the remote host
at System.Net.Sockets.Socket.Send(Byte[] buffer, Int32 offset, Int32 size, SocketFlags socketFlags)
at System.Net.Sockets.NetworkStream.Write(Byte[] buffer, Int32 offset, Int32 size)
--- End of inner exception stack trace ---
at System.Net.Sockets.NetworkStream.Write(Byte[] buffer, Int32 offset, Int32 size)
at System.IO.BufferedStream.Flush()
at System.IO.StreamWriter.Flush(Boolean flushStream, Boolean flushEncoder)
at System.IO.StreamWriter.Flush()
at ScriptSharp.Testing.WebServer.HttpMessage.WriteStatus(HttpStatusCode statusCode)
at ScriptSharp.Testing.WebServer.HttpMessage.Run()
at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.ThreadHelper.ThreadStart()
"""

[nikhilk]

I suspect this change in the test scripts likely address this by preventing the browser from closing the connection prematurely before the server has a chance to write out its http response:

5455bf8#src/ZipX/ProjectTemplates/UnitTest/QUnitExt.js

Please try that change locally (just copy the additions into your copy of QUnitExt.js in your test project) and report back if it issue still persists.

[sucaba]

Actually I'm already using the most recent version of QUnitExt.js.
Problem persists both with and without this change.

[nikhilk]

I haven't seen this running tests inside of visual studio, so don't know if it stems from running with nunit. So if you can make a change and try, here is something to try:

Try moving https://github.com/nikhilk/scriptsharp/blob/master/src/Tools/Testing/WebServer/WebTestHttpServer.cs#L100 this to write out http status before raising the log event (which initiates the end of the test).

I should probably make that change anyway, but it would be interesting to verify in your setup whether it has a bearing on the issue or not.

[sucaba]

After debugging a bit, it appears that NUnit worker process didn't close because HttpServer thread was not closed.
Problem is that listener.AcceptTcpClient() is blocking method and setting _alive to false didn't stop listener loop. I made code a bit safer by stopping listener outside the thread and additionally stopping thread explicitly after that. Quick and dirty, but it works for TestDriven.Net & NUnit. Please let me know if this change is ok to you. Here is output of "git diff":

diff --git a/src/Tools/Testing/WebServer/HttpServer.cs b/src/Tools/Testing/WebServer/HttpServer.cs
index b0f17dd..25fc526 100644
--- a/src/Tools/Testing/WebServer/HttpServer.cs
+++ b/src/Tools/Testing/WebServer/HttpServer.cs
@@ -15,6 +15,8 @@ namespace ScriptSharp.Testing.WebServer {
         private int _port;
         private Action<HttpMessage> _getHandler;
         private Action<HttpMessage> _postHandler;
+        private TcpListener listener;
+        private Thread mainThread;

         private bool _alive;

@@ -24,11 +26,12 @@ namespace ScriptSharp.Testing.WebServer {
         }

         private void Run() {
-            TcpListener listener = new TcpListener(new IPEndPoint(IPAddress.Loopback, _port));
+            this.listener = new TcpListener(new IPEndPoint(IPAddress.Loopback, _port));
             listener.Start();

             while (_alive) {
-                try {
+                try
+                {
                     TcpClient client = listener.AcceptTcpClient();

                     HttpMessage message = new HttpMessage(this, _getHandler, _postHandler);
@@ -51,12 +54,17 @@ namespace ScriptSharp.Testing.WebServer {
             _alive = true;
             _port = port;

-            Thread mainThread = new Thread(Run);
+            this.mainThread = new Thread(Run);
             mainThread.Start();
         }

         public void Stop() {
             _alive = false;
+            listener.Stop();
+            if (!mainThread.Join(1000))
+            {
+                mainThread.Abort();
+            }
         }
     }
 }

[nikhilk]

Looks reasonable ... will incorporate into the branch with changes for next release. Thanks for the investigation.

[nikhilk]

Quick question - do you think its better to simply only call AcceptTcpClient if there is a pending connection, and leave the rest unchanged?

[sucaba]

I think that error handling can be improved (currently all exceptions are suppressed). For quick solution all exception swallowing is fine. However bigger projects running thousand of tests every day can provoke many tcp or http error situations. So, your question is probably about quality level.
For nearest release it would be good to:

1. Log unexpected errors using Debug.Write or to Windows Event log (note that problems may occur during continuous integration pass on another server, then logging may help to find out what went wrong).
2. Also it is not clear whether there are any exceptions which can be simply swallowed without stopping server. Maybe it is worth to stop HTTP server on any exception from AcceptTcpClient? Have you found any problems around this?

BTW, at this moment AcceptTcpClient throws exception when stopping listener from the owning thread. So, there is no need to log socket exception when _alive == false.

I like the idea of having custom lightweight HTTP server, but IMO it will take some time to make it mature. Let me know it you need any help around this.

[nikhilk]

I think #1 is fine in terms of Debug.Write logging to help debug while working on the test server itself, as people are unlikely to use the debug bits in their own test projects. For actual error reporting, would need to think about plugging that into the test error reporting rather than Debug.Write or some other logging system.

#2 - don't want potential exception setting up the thread to process a single request to stop the server. If at all, it should fail the test being run, rather than just stopping the server.

In general, the idea is to always improve, but improve so it meets the needs of the scenario - a test server that serves up static files and records logs, rather than a general purpose web server. In an ideal world simply opening up test html files/scripts in browser directly from the file system would suffice for testing client functionality... but the requirement to connect to the vs test infrastructure to log results creates the need for the web server.

For now, I'll make the change to call AcceptTcpClient only when Pending() returns true, so that the call doesn't block, which should then let the thread end when _alive is set to false.

[sucaba]

After running tests today I discovered that both my fix and the one based on the Pending() check are fine for fixing hang problem. Unfortunately initial exception persists even though has no effect on test run itself. It is something related to processing HTTP messages in parallel threads. I will create new bug once I understand the solution for this.