You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by mb...@apache.org on 2003/02/21 14:48:09 UTC

cvs commit: jakarta-commons/httpclient/src/java/org/apache/commons/httpclient HttpMethodBase.java HttpClient.java

mbecke      2003/02/21 05:48:09

  Modified:    httpclient/src/java/org/apache/commons/httpclient
                        HttpMethodBase.java HttpClient.java
  Log:
  Fixed a bug where an HttpConnection could be lost.  If an exception occurred between the point when a connection was retrieved and when is was assigned to a method the connection would be lost.
PR:
Obtained from: Sam Maloney
  
  Revision  Changes    Path
  1.115     +8 -5      jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java
  
  Index: HttpMethodBase.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpMethodBase.java,v
  retrieving revision 1.114
  retrieving revision 1.115
  diff -u -r1.114 -r1.115
  --- HttpMethodBase.java	18 Feb 2003 15:54:29 -0000	1.114
  +++ HttpMethodBase.java	21 Feb 2003 13:48:09 -0000	1.115
  @@ -930,6 +930,10 @@
                   
           LOG.trace("enter HttpMethodBase.execute(HttpState, HttpConnection)");
   
  +        // this is our connection now, assign it to a local variable so 
  +        // that it can be released later
  +        this.responseConnection = conn;
  +
           checkExecuteConditions(state, conn);
           inExecute = true;
   
  @@ -946,7 +950,6 @@
   
               while (forwardCount++ < MAX_FORWARDS) {
                   // on every retry, reset this state information.
  -                responseConnection = conn;
                   conn.setLastResponseInputStream(null);
   
                   if (LOG.isDebugEnabled()) {
  
  
  
  1.69      +25 -11    jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpClient.java
  
  Index: HttpClient.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/HttpClient.java,v
  retrieving revision 1.68
  retrieving revision 1.69
  diff -u -r1.68 -r1.69
  --- HttpClient.java	28 Jan 2003 04:40:20 -0000	1.68
  +++ HttpClient.java	21 Feb 2003 13:48:09 -0000	1.69
  @@ -84,6 +84,7 @@
    * @author Ortwin Gl�ck
    * @author <a href="mailto:becke@u.washington.edu">Michael Becke</a>
    * @author <a href="mailto:mbowler@GargoyleSoftware.com">Mike Bowler</a>
  + * @author Sam Maloney
    * 
    * @version $Revision$ $Date$
    */
  @@ -550,15 +551,28 @@
               httpConnectionTimeout
           );
   
  -        method.setStrictMode(strictMode);
  +        try {
  +            // Catch all possible exceptions to make sure to release the 
  +            // connection, as although the user may call 
  +            // Method->releaseConnection(), the method doesn't know about the
  +            // connection until HttpMethod.execute() is called.
  +            
  +            method.setStrictMode(strictMode);
           
  -        if (!connection.isOpen()) {
  -            connection.setSoTimeout(soTimeout);
  -            connection.setConnectionTimeout(connectionTimeout);
  -            connection.open();
  -            if (connection.isProxied() && connection.isSecure()) {
  -                method = new ConnectMethod(method);
  +            if (!connection.isOpen()) {
  +                connection.setSoTimeout(soTimeout);
  +                connection.setConnectionTimeout(connectionTimeout);
  +                connection.open();
  +                if (connection.isProxied() && connection.isSecure()) {
  +                    method = new ConnectMethod(method);
  +                }
               }
  +        } catch (IOException e) {
  +            connection.releaseConnection();
  +            throw e;
  +        } catch (RuntimeException e) {
  +            connection.releaseConnection();
  +            throw e;
           }
           
           return method.execute(state, connection);
  
  
  

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: cvs commit: jakarta-commons/httpclient/src/java/org/apache/commons/httpclient HttpMethodBase.java HttpClient.java

Posted by Ortwin Glück <or...@nose.ch>.
mbecke@apache.org wrote:
>   +        } catch (IOException e) {
>   +            connection.releaseConnection();
>   +            throw e;
>   +        } catch (RuntimeException e) {
>   +            connection.releaseConnection();
>   +            throw e;
>            }

I suggest using a finally clause instead of the above construct. It's no 
good catching a RuntimeException (you may easily overlook a NPE that 
stems from a programming error). See [1] for details.

[1] http://www.javaworld.com/javaworld/javatips/jw-javatip134.html


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org