hc-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eric Johnson <e...@tibco.com>
Subject [PATCH] close on input streams handled properly - redux
Date Wed, 04 Dec 2002 22:44:16 GMT
All right, another try, dealing with the complexities of Michael's changes.

This patch is a top-to-bottom revisting of my previous patch.  Highlights:

    * As before AutoCloseInputStream, ContentLengthInputStream, and
      ChunkedInputStream no longer "close" their wrapped stream.  Now,
      however, when the content is consumed, an optional
      ResponseConsumedWatcher will be notified via the function
    * New interface ResponseConsumedWatcher - so named as I didn't want
      to use the term "Listener", which usually implies in Java that
      there can be multiple "listeners".  In this case, there can be
      only one "watcher", and it may optionally be provided for the
      constructor.  The interface is public only because the various
      input stream wrappers are also public.
    * The function HttpMethodBase.wrapResponseStream() is no longer
      needed, and neither is its corresponding class
    * As a result of the preceding two changes, any time a response body
      is consumed, it is funnelled through the responseBodyConsumed()
      function, meaning there is just one(!) place to go to look for the
      logic as to what happens at the end of a response.  You no longer
      need to look at the interaction of AutoCloseInputStream,
      ContentLengthInputStream, and ResponseAutoReleaseInputStream.
    * MultiThreadedHttpConnectionManager.releaseConnection() - now
      "finishes" the previous response before adding a connection to its
    * SimpleHttpConnectionManager.releaseConnection - finishes previous
      response and insures that the connection being released matches
      the connection it is tracking.  Also "finishes" a response prior
      to returning it, as per email that Michael and I sent to this group.
    * No changes to HttpClient class!
    * HttpMethodBase.releaseConnection() function merely calls "close"
      on the responseStream - which triggers a responseConsumed() call.
    * HttpMethodBase.execute() - now has a single exit point, and has
      been subdivided into three functions.  The execute() function
      itself is wrapped in a try/finally block to insure that state is
      correct at the end of the execute call.

It turns out that there were some additional corner cases that I missed 
previously with regards to my previous patch.  I think I've simplified 
the model now, and guaranteed that all the cases get caught.  Central to 
this change is the single exit point for the execute() function.

The execute() function now sets internal state indicating that a 
connection should be released when the execute() call is done.  At the 
beginning of reading the response body, this flag gets reset.  At any 
point after that (still in the execute() function), should the 
responseConsumed() function be invoked, the flag will be set to 
"release" the connection when execute is done.  If execute() completes, 
and the connection needs to remain open, and responseConsumed() is 
subsequently triggered, the connection will then be released.  If the 
response is never closed, or read in its entirety, one of two things 
will happen.  When using SimpleHttpConnectionManager, a subsequent 
request will close the previous response.  When using the 
MultiThreadedHttpConnectionManager, the response will remain open until 
garbage collected (or consumed, or closed, of course).

An explicit call to releaseConnection() on the method simply closes the 
input stream, which cascades to closing the connection (when called 
outside of execute).

One minor subtlety, the responseConsumed() function can be invoked 
multiple times with a single response, particularly when an 
AutoCloseInputStream wraps a ContentLengthInputStream or 
ChunkedInputStream (that is to say, for example, a response that 
includes both a "content-length" header and a "connection: close" 
header).  The code accommodates this detail, but I wish it was cleaner.

I hope this patch passes muster this time around!  I appreciate 
everyone's patience.

-Eric Johnson

View raw message