KristianRosenvold: Made killable logic only kill browser, not interru…

…pt thread

Using any kind of interrupted logic to detect session timeout seems like the Wrong Thing To Do

r16454

java/client/src/org/openqa/selenium/remote/ErrorCodes.java

@@ -56,7 +56,6 @@ public class ErrorCodes {
   public static final int INVALID_XPATH_SELECTOR = 51;
   public static final int INVALID_XPATH_SELECTOR_RETURN_TYPER = 52;
   // The following error codes are derived straight from HTTP return codes.
-  public static final int GONE = 404;
   public static final int METHOD_NOT_ALLOWED = 405;
 
   /**

java/client/src/org/openqa/selenium/remote/HttpCommandExecutor.java

@@ -315,7 +315,7 @@ private HttpResponse fallBackExecute(HttpContext context, HttpUriRequest httpMet
       try {
         Thread.sleep(2000);
       } catch (InterruptedException ie) {
-        throw new SessionTerminatedException();
+        throw Throwables.propagate(ie);
       }
     }
     return client.execute(targetHost, httpMethod, context);

java/server/src/org/openqa/grid/internal/SessionTerminationReason.java

@@ -25,5 +25,6 @@ public enum SessionTerminationReason {
   CLIENT_GONE,
   FORWARDING_TO_NODE_FAILED,
   CREATIONFAILED,
-  PROXY_REREGISTRATION
-}
+  PROXY_REREGISTRATION,
+  SO_TIMEOUT
+  }

java/server/src/org/openqa/grid/internal/TestSession.java

@@ -192,6 +192,7 @@ private HttpClient getClient() {
     if (browserTimeout > 0){
       final int selenium_server_cleanup_cycle = browserTimeout / 10;
       browserTimeout += (selenium_server_cleanup_cycle + MAX_NETWORK_LATENCY);
+      browserTimeout *=2; // Lets not let this happen too often
     }
     return slot.getProxy().getHttpClientFactory().getGridHttpClient(browserTimeout);
   }

java/server/src/org/openqa/grid/web/servlet/handler/RequestHandler.java

@@ -18,6 +18,7 @@
 package org.openqa.grid.web.servlet.handler;
 
 import java.io.IOException;
+import java.net.SocketTimeoutException;
 import java.util.Map;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -127,6 +128,8 @@ public void process() {
         } catch (ClientGoneException e) {
           log.log(Level.WARNING, "The client is gone for session " + session + ", terminating");
           registry.terminate(session, SessionTerminationReason.CLIENT_GONE);
+        } catch (SocketTimeoutException e){
+          registry.terminate(session, SessionTerminationReason.SO_TIMEOUT);
         } catch (Throwable t) {
           log.log(Level.SEVERE, "cannot forward the request " + t.getMessage(), t);
           registry.terminate(session, SessionTerminationReason.FORWARDING_TO_NODE_FAILED);

java/server/src/org/openqa/selenium/remote/server/SessionCleaner.java

@@ -44,6 +44,10 @@ class SessionCleaner extends Thread {   // Thread safety reviewed
     if (clientGoneTimeout == 0 && insideBrowserTimeout == 0){
       throw new IllegalStateException("SessionCleaner not supposed to start when no timeouts specified");
     }
+    if (insideBrowserTimeout > 0 && insideBrowserTimeout < 60000){
+      log.warning("The specified browser timeout is TOO LOW for safe operations and may have"+
+                  "other side-effects\n. Please specify a slightly higher browserTimeout.");
+    }
     int lowestNonZero = Math.min((insideBrowserTimeout > 0) ? insideBrowserTimeout : clientGoneTimeout,
                                   clientGoneTimeout > 0 ? clientGoneTimeout : insideBrowserTimeout);
     this.sleepInterval = lowestNonZero / 10;
@@ -89,7 +93,7 @@ void checkExpiry() {
             driver = ((EventFiringWebDriver)driver).getWrappedDriver();
           }
           if (driver instanceof Killable) {
-            session.interrupt();
+            //session.interrupt();
             ((Killable) driver).kill();
             killed = true;
             log.warning("Browser killed and session " + session.getSessionId() + " terminated due to in-browser timeout.");