Skip to content
This repository has been archived by the owner on Nov 29, 2018. It is now read-only.

Selenium Grid availability - HttpConnection leakage problem #6773

Closed
lukeis opened this issue Mar 4, 2016 · 8 comments
Closed

Selenium Grid availability - HttpConnection leakage problem #6773

lukeis opened this issue Mar 4, 2016 · 8 comments
Assignees
@lukeis
Labels
Component-Grid Priority-Medium Restrict-AddIssueComment-Commit Status-Fixed Type-Defect

Comments

@lukeis
Copy link
Member

lukeis commented Mar 4, 2016

Originally reported on Google Code with ID 6773

we confronted the HttPConnection leakage problem. And I made below fixes. They are proved
working for 100 hours testing. Could you please review them as well, and fix them in
the next version?
TestSession.java.

  public boolean sendDeleteSessionRequest() {
         …
    HttpHost host = new HttpHost(remoteURL.getHost(), remoteURL.getPort());
    HttpEntity responseBody = null;
    boolean ok;
    try {
      HttpClient client = getClient();
      HttpResponse response = client.execute(host, request);
      responseBody = response.getEntity();
      int code = response.getStatusLine().getStatusCode();
      ok = (code >= 200) && (code <= 299);
    } catch (Throwable e) {
      ok = false;
      // corrupted or the something else already sent the DELETE.
      log.severe("Error releasing. Server corrupted ?");
    }finally{//Add to release connection finally by Jojo
    try {
      EntityUtils.consume(responseBody);
   } catch (IOException e) {
      // TODO Auto-generated catch block
      e.printStackTrace();
   }

Note: This handle for sendDeleteSessionRequest is necessary. Without this handling,
httpConnection leakage problem will happen.
Below changes havn’t proved the necessity. Just add the handle to ensure closing the
stream.

public String forward(SeleniumBasedRequest request, HttpServletResponse response,
                        boolean newSessionRequest)
      throws IOException {
….
    try {
      …

      HttpResponse proxyResponse = sendRequestToNode(proxyRequest);
      lastActivity = timeSource.currentTimeInMillis();
      HttpEntity responseBody = proxyResponse.getEntity();//Move up by Jojo to release
connection thoroughly
      try{
           ….
          //HttpEntity responseBody = proxyResponse.getEntity();
          byte[] contentBeingForwarded = null;
          if (responseBody != null) {
            try {
              InputStream in;
             …
          response.flushBuffer();
      }finally{//Add to release connection finally by Jojo
        EntityUtils.consume(responseBody);
      }
return res;

    } finally {
      forwardingRequest = false;
      Thread.currentThread().setName(currentThreadName);
    }
  }

BaseRemoteProxy.java
public JSONObject getStatus() throws GridException {
。。。
    String existingName = Thread.currentThread().getName();
    HttpEntity entity = null;
    try {
      Thread.currentThread().setName("Probing status of " + url);
      response = client.execute(host, r);
      entity = response.getEntity();
      int code = response.getStatusLine().getStatusCode();
。。。

    } catch (Exception e) {
      throw new GridException(e.getMessage(), e);
    } finally {
      Thread.currentThread().setName(existingName);
      try { //Added by jojo to release connection thoroughly
      EntityUtils.consume(entity);
      } catch (IOException e) {
        log.info("Exception thrown when consume entity");
      }
    }
  }

Reported by shijunjuan on 2013-12-26 07:53:20
@lukeis
Copy link
Member Author

lukeis commented Mar 4, 2016 

When we met this problem, eventually, the grid can't create http connection even no
 established connection at all. And all threads are stuck in this step:

"Forwarding fc21e947-f9d7-4f76-92d6-171604ce2a40 (int. key, remote not contacted yet.)
to http://10.9.245.140:5555/wd/hub at 12:41:01 AM" - Thread t@119093
      java.lang.Thread.State: TIMED_WAITING
         at sun.misc.Unsafe.park(Native Method)
         - parking to wait for <5f4eaa76> (a java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject)
         at java.util.concurrent.locks.LockSupport.parkUntil(LockSupport.java:237)
         at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.awaitUntil(AbstractQueuedSynchronizer.java:2072)
         at org.apache.http.impl.conn.tsccm.WaitingThread.await(WaitingThread.java:157)
         at org.apache.http.impl.conn.tsccm.ConnPoolByRoute.getEntryBlocking(ConnPoolByRoute.java:398)
         at org.apache.http.impl.conn.tsccm.ConnPoolByRoute$1.getPoolEntry(ConnPoolByRoute.java:298)
         at org.apache.http.impl.conn.tsccm.ThreadSafeClientConnManager$1.getConnection(ThreadSafeClientConnManager.java:238)
         at org.apache.http.impl.client.DefaultRequestDirector.execute(DefaultRequestDirector.java:422)
         at org.apache.http.impl.client.AbstractHttpClient.doExecute(AbstractHttpClient.java:863)
         at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:115)
         at org.apache.http.impl.client.CloseableHttpClient.execute(CloseableHttpClient.java:57)
         at org.openqa.grid.internal.TestSession.sendRequestToNode(TestSession.java:360)
         at org.openqa.grid.internal.TestSession.forward(TestSession.java:222)
         at org.openqa.grid.web.servlet.handler.RequestHandler.forwardNewSessionRequestAndUpdateRegistry(RequestHandler.java:87)
         at org.openqa.grid.web.servlet.handler.RequestHandler.process(RequestHandler.java:109)
         at org.openqa.grid.web.servlet.DriverServlet.process(DriverServlet.java:84)
         at org.openqa.grid.web.servlet.DriverServlet.doPost(DriverServlet.java:68)
         at javax.servlet.http.HttpServlet.service(HttpServlet.java:727)
         at javax.servlet.http.HttpServlet.service(HttpServlet.java:820)
         at org.seleniumhq.jetty7.servlet.ServletHolder.handle(ServletHolder.java:565)
         at org.seleniumhq.jetty7.servlet.ServletHandler.doHandle(ServletHandler.java:479)
         at org.seleniumhq.jetty7.server.session.SessionHandler.doHandle(SessionHandler.java:225)
         at org.seleniumhq.jetty7.server.handler.ContextHandler.doHandle(ContextHandler.java:1031)
         at org.seleniumhq.jetty7.servlet.ServletHandler.doScope(ServletHandler.java:406)
         at org.seleniumhq.jetty7.server.session.SessionHandler.doScope(SessionHandler.java:186)
         at org.seleniumhq.jetty7.server.handler.ContextHandler.doScope(ContextHandler.java:965)
         at org.seleniumhq.jetty7.server.handler.ScopedHandler.handle(ScopedHandler.java:117)
         at org.seleniumhq.jetty7.server.handler.HandlerWrapper.handle(HandlerWrapper.java:111)
         at org.seleniumhq.jetty7.server.Server.handle(Server.java:349)
         at org.seleniumhq.jetty7.server.AbstractHttpConnection.handleRequest(AbstractHttpConnection.java:452)
         at org.seleniumhq.jetty7.server.BlockingHttpConnection.handleRequest(BlockingHttpConnection.java:47)
         at org.seleniumhq.jetty7.server.AbstractHttpConnection.content(AbstractHttpConnection.java:894)
         at org.seleniumhq.jetty7.server.AbstractHttpConnection$RequestHandler.content(AbstractHttpConnection.java:948)
         at org.seleniumhq.jetty7.http.HttpParser.parseNext(HttpParser.java:857)
         at org.seleniumhq.jetty7.http.HttpParser.parseAvailable(HttpParser.java:235)
         at org.seleniumhq.jetty7.server.BlockingHttpConnection.handle(BlockingHttpConnection.java:66)
         at org.seleniumhq.jetty7.server.bio.SocketConnector$ConnectorEndPoint.run(SocketConnector.java:254)
         at org.seleniumhq.jetty7.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:599)
         at org.seleniumhq.jetty7.util.thread.QueuedThreadPool$3.run(QueuedThreadPool.java:534)
         at java.lang.Thread.run(Thread.java:662)

Reported by shijunjuan on 2013-12-26 08:13:58

@lukeis lukeis self-assigned this Mar 4, 2016
@lukeis
Copy link
Member Author

lukeis commented Mar 4, 2016

Reported by barancev on 2013-12-27 10:00:38

  • Labels added: Component-Grid

@lukeis
Copy link
Member Author

lukeis commented Mar 4, 2016 

Thanks for tracking this down. To have your contribution accepted, please: 

a. Make sure you've signed the CLA at http://goo.gl/qC50R
b. Create a *patch* against the current master and attach it to this issue (the output
of `git diff` after making the necessary changes is fine).

Reported by jari.bakken on 2014-01-28 20:45:18

  • Status changed: NeedsClarification
  • Labels removed: Status-Untriaged

@lukeis
Copy link
Member Author

lukeis commented Mar 4, 2016 

diff --git a/java/server/src/org/openqa/grid/internal/BaseRemoteProxy.java b/java/server/src/org/openqa/grid/internal/BaseRemoteProxy.java
index c103f9e..3c18bd6 100644
--- a/java/server/src/org/openqa/grid/internal/BaseRemoteProxy.java
+++ b/java/server/src/org/openqa/grid/internal/BaseRemoteProxy.java
@@ -22,6 +22,7 @@
 import static org.openqa.grid.common.RegistrationRequest.REMOTE_HOST;
 import static org.openqa.grid.common.RegistrationRequest.SELENIUM_PROTOCOL;

+import org.apache.http.HttpEntity;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
@@ -502,10 +503,11 @@ public JSONObject getStatus() throws GridException {
     HttpHost host = new HttpHost(getRemoteHost().getHost(), getRemoteHost().getPort());
     HttpResponse response;
     String existingName = Thread.currentThread().getName();
-
+    HttpEntity entity = null;
     try {
       Thread.currentThread().setName("Probing status of " + url);
       response = client.execute(host, r);
+      entity = response.getEntity();
       int code = response.getStatusLine().getStatusCode();

       if (code == 200) {
@@ -530,6 +532,12 @@ public JSONObject getStatus() throws GridException {
       throw new GridException(e.getMessage(), e);
     } finally {
       Thread.currentThread().setName(existingName);
+      try { //Added by jojo to release connection thoroughly
+          EntityUtils.consume(entity);
+          } catch (IOException e) {
+            log.info("Exception thrown when consume entity");
+          }
+
     }
   }

diff --git a/java/server/src/org/openqa/grid/internal/TestSession.java b/java/server/src/org/openqa/grid/internal/TestSession.java
index a363fcc..7d18e16 100644
--- a/java/server/src/org/openqa/grid/internal/TestSession.java
+++ b/java/server/src/org/openqa/grid/internal/TestSession.java
@@ -226,57 +226,62 @@ public String forward(SeleniumBasedRequest request, HttpServletResponse
response

       HttpResponse proxyResponse = sendRequestToNode(proxyRequest);
       lastActivity = timeSource.currentTimeInMillis();
-
-      final int statusCode = proxyResponse.getStatusLine().getStatusCode();
-      response.setStatus(statusCode);
-      processResponseHeaders(request, response, slot.getRemoteURL(), proxyResponse);
-
-      byte[] consumedNewWebDriverSessionBody = null;
-      if (statusCode != HttpServletResponse.SC_INTERNAL_SERVER_ERROR &&
-          statusCode != HttpServletResponse.SC_NOT_FOUND) {
-        consumedNewWebDriverSessionBody = updateHubIfNewWebDriverSession(request,
proxyResponse);
-      }
-      if (newSessionRequest && statusCode == HttpServletResponse.SC_INTERNAL_SERVER_ERROR)
{
-        removeIncompleteNewSessionRequest();
-      }
-      if (statusCode == HttpServletResponse.SC_NOT_FOUND) {
-        removeSessionBrowserTimeout();
-      }
-
-      HttpEntity responseBody = proxyResponse.getEntity();
-      byte[] contentBeingForwarded = null;
-      if (responseBody != null) {
-        try {
-          InputStream in;
-          if (consumedNewWebDriverSessionBody == null) {
-            in = responseBody.getContent();
-            if (request.getRequestType() == RequestType.START_SESSION
-                && request instanceof LegacySeleniumRequest) {
-              res = getResponseUtf8Content(in);
-
-              updateHubNewSeleniumSession(res);
-
-              in = new ByteArrayInputStream(res.getBytes("UTF-8"));
-            }
-          } else {
-            in = new ByteArrayInputStream(consumedNewWebDriverSessionBody);
-          }
-
-          final byte[] bytes = drainInputStream(in);
-          writeRawBody(response, bytes);
-
-        } finally {
+      HttpEntity responseBody = proxyResponse.getEntity();//Move up by Jojo to release
connection thoroughly
+      try{
+         final int statusCode = proxyResponse.getStatusLine().getStatusCode();
+         response.setStatus(statusCode);
+         processResponseHeaders(request, response, slot.getRemoteURL(), proxyResponse);
+   
+         byte[] consumedNewWebDriverSessionBody = null;
+         if (statusCode != HttpServletResponse.SC_INTERNAL_SERVER_ERROR &&
+             statusCode != HttpServletResponse.SC_NOT_FOUND) {
+           consumedNewWebDriverSessionBody = updateHubIfNewWebDriverSession(request,
proxyResponse);
+         }
+         if (newSessionRequest && statusCode == HttpServletResponse.SC_INTERNAL_SERVER_ERROR)
{
+           removeIncompleteNewSessionRequest();
+         }
+         if (statusCode == HttpServletResponse.SC_NOT_FOUND) {
+           removeSessionBrowserTimeout();
+         }
+   
+         //HttpEntity responseBody = proxyResponse.getEntity();
+         byte[] contentBeingForwarded = null;
+         if (responseBody != null) {
+           try {
+             InputStream in;
+             if (consumedNewWebDriverSessionBody == null) {
+               in = responseBody.getContent();
+               if (request.getRequestType() == RequestType.START_SESSION
+                   && request instanceof LegacySeleniumRequest) {
+                 res = getResponseUtf8Content(in);
+   
+                 updateHubNewSeleniumSession(res);
+   
+                 in = new ByteArrayInputStream(res.getBytes("UTF-8"));
+               }
+             } else {
+               in = new ByteArrayInputStream(consumedNewWebDriverSessionBody);
+             }
+   
+             final byte[] bytes = drainInputStream(in);
+             writeRawBody(response, bytes);
+   
+           } finally {
+             EntityUtils.consume(responseBody);
+           }
+   
+         }
+   
+         if (slot.getProxy() instanceof CommandListener) {
+           SeleniumBasedResponse wrappedResponse = new SeleniumBasedResponse(response);
+           wrappedResponse.setForwardedContent(contentBeingForwarded);
+           ((CommandListener) slot.getProxy()).afterCommand(this, request, wrappedResponse);
+         }
+         response.flushBuffer();
+      }finally{//Add to release connection finally by Jojo
           EntityUtils.consume(responseBody);
         }

-      }
-
-      if (slot.getProxy() instanceof CommandListener) {
-        SeleniumBasedResponse wrappedResponse = new SeleniumBasedResponse(response);
-        wrappedResponse.setForwardedContent(contentBeingForwarded);
-        ((CommandListener) slot.getProxy()).afterCommand(this, request, wrappedResponse);
-      }
-      response.flushBuffer();
       return res;
     } finally {
       forwardingRequest = false;
@@ -544,17 +549,24 @@ public boolean sendDeleteSessionRequest() {
     }

     HttpHost host = new HttpHost(remoteURL.getHost(), remoteURL.getPort());
-
+    HttpEntity responseBody = null;
     boolean ok;
     try {
       HttpClient client = getClient();
       HttpResponse response = client.execute(host, request);
+      responseBody = response.getEntity();
       int code = response.getStatusLine().getStatusCode();
       ok = (code >= 200) && (code <= 299);
     } catch (Throwable e) {
       ok = false;
       // corrupted or the something else already sent the DELETE.
       log.severe("Error releasing. Server corrupted ?");
+    }finally{//Add to release connection finally by Jojo
+        try {
+            EntityUtils.consume(responseBody);
+         } catch (IOException e) {
+            e.printStackTrace();
+         }
     }
     return ok;
   }

Reported by shijunjuan on 2014-01-29 06:53:19

@lukeis
Copy link
Member Author

lukeis commented Mar 4, 2016 

I have signed the CLA and above is the output of 'git diff' command.

Reported by shijunjuan on 2014-01-29 06:54:37

@lukeis
Copy link
Member Author

lukeis commented Mar 4, 2016 

Fixed by revision 21bc4f708f

Reported by barancev on 2014-06-30 22:11:58

  • Status changed: Fixed

@lukeis
Copy link
Member Author

lukeis commented Mar 4, 2016

Reported by jari.bakken on 2015-02-26 23:02:13

@lukeis
Copy link
Member Author

lukeis commented Mar 4, 2016

Reported by luke.semerau on 2015-09-17 18:21:53

  • Labels added: Restrict-AddIssueComment-Commit

@lukeis lukeis closed this as completed Mar 4, 2016
@SeleniumHQ SeleniumHQ locked and limited conversation to collaborators Mar 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@lukeis lukeis

Labels
Component-Grid Priority-Medium Restrict-AddIssueComment-Commit Status-Fixed Type-Defect
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@lukeis