I agree with you this feature is strange and need not be added to the library. As I said before, I need to preserve this feature for backwards compatibility. It has something to do with delaying the emission of an MDN in an AS2 gateway so it represents the status of the receiver instead of the man in the middle. Again, please don't ask me to change the architecture of the solution. We don't own the application and we can't change it. Nevertheless it must continue to work after we port to HttpClient 4.5.2
As for the work-around, I have something working but it duplicates too much code for my taste. I think this is a nice opportunity for HttpClient to learn what it is like to try to subclass its engine. I am subclassing HttpClientBuilder to override createMainExec() to return my own implementation of the ClientExecChain. This is nice. Unfortunately, our subclass of ClientExecChain must be in the org.apache.http.impl.execchain package because support classes in that package are all package private. This is annoying. I modified the problematic code in ClientExecChain.execute() but that was not enough. I need to trap the EofSensorWatcher of the response HttpEntity. That requires duplicating HttpResponseProxy and ResponseEntityProxy. Again this must be done in the apache package. The release should go through the ConnectionHolder since that's where the "AtomicBoolean released" field is stored. I thought of providing my own implementation of ConnectionHolder with an empty releaseConnection() method, but that does not work when ResponseEntityProxy calls cleanup(), this becomes releaseConnection(false) which closes the connection completely. I would appreciate if you have better suggestions for the work-around. It would be nice to make subclassing easier too. By the way, should ResponseEntityProxy.enchance() be called enhance() instead? Compare this to RequestEntityProxy.enhance() -----Original Message----- From: Oleg Kalnichevski [mailto:ol...@apache.org] Sent: Tuesday, October 18, 2016 10:48 AM To: HttpClient User Discussion Subject: Re: Controlling releaseConnection On Tue, 2016-10-18 at 10:12 -0400, Pellerin, Clement wrote: > I agree that code is correct. I never said there was a bug. > I am asking how to override the behavior. > We are porting our product from HttpClient 4.1 to 4.5.2 and we need to > preserve that feature because it is used by our customers. Clement I'll be happy to suggest a work-around but I am still unable to understand what it is exactly you are trying to achieve by keeping a connection for messages without a message body, which is quite uncommon (status 204, status 304, response to HEAD, and that is it). What is exactly the point of doing so in th first place? Oleg > It would be a shame to duplicate all of MainClientExec because of a > lack of modularity in that class. > > We tried to subclass the PoolingHttpClientConnectionManager > but that does not work because the connection state is actually stored > in the ConnectionHolder created directly in MainClientExec.execute(). > final ConnectionHolder connHolder = new > ConnectionHolder(this.log, this.connManager, managedConn); > > Basically, I'm asking if the maintainers could consider a method like > protected boolean isEarlyRelease(HttpEntity entity) { > return entity == null || !entity.isStreaming(); > } > > Or a release strategy interface if this is a common requirement. > I am also interested in any other work-around. > > -----Original Message----- > From: Shawn Heisey [mailto:apa...@elyograg.org] > Sent: Monday, October 17, 2016 9:24 PM > To: HttpClient User Discussion > Subject: Re: Controlling releaseConnection > > On 10/17/2016 3:22 PM, Pellerin, Clement wrote: > > Our customer needs to delay the release of the connection until the > > response is fully processed. > > They want to turn off the early automatic release of the connection and do > > it manually later. > > > > This is the problematic code in MainClientExec > > // check for entity, release connection if possible > > final HttpEntity entity = response.getEntity(); > > if (entity == null || !entity.isStreaming()) { > > // connection not needed and (assumed to be) in re-usable > > state > > connHolder.releaseConnection(); > > return new HttpResponseProxy(response, null); > > } else { > > return new HttpResponseProxy(response, connHolder); > > } > > Mostly an end-user here, with no status to speak of in this project. I do > have status on another Apache project that utilizes HttpClient, but I don't > know much about that part of the code. I have written some HttpClient code > for a completely unrelated project of my own, but that code is VERY simple. > > When I read the code above, what I see is this: It only releases the > connection if the entity is nonexistent (null) or the entity is NOT a type > that uses streaming. > > I will fully admit that my experience with HttpClient is limited, but I think > the chance is very small that the HttpComponents committers have made a > mistake here. I think this particular code has probably been discussed and > examined, then ultimately validated as correct. Here's why I think they > didn't make a mistake: > > If the entity object is null, then the response probably doesn't HAVE an > entity (response body), so it will be entirely self-contained, consisting of > headers only, and the connection doesn't have anything further to send. If > the entity exists but doesn't utilize streaming, then I think it's likely > that the entity was received in its entirety and has been incorporated into > the response object already, and once again, the connection isn't needed. If > my limited understanding of non-streaming entities is correct, they have the > potential to be very dangerous from a memory consumption perspective, and my > own usage of HttpClient (where I did not set anything related to the entity > type) suggests that streaming entities are used by default. > > Restating in another way: In the first situation that results in a released > connection, there's nothing to consume, you just need the response object > that you already have. In the second situation, the entity you will consume > is probably already available within the response object and doesn't need the > connection. The comment on the release call in the code quoted above implies > that this is how things work. > > In these situations, why do you need the connection to stick around? I think > it can't do anything else that's useful for that request. I would imagine > that if the connection utilizes keepalive/pipelining, that it will typically > remain open after release and can be utilized again for a different request. > > Someone with more direct knowledge of HttpClient's internal implementation > will need to confirm whether or not I'm correct in what I've written. My > understanding could be wrong. > > Thanks, > Shawn --------------------------------------------------------------------- To unsubscribe, e-mail: httpclient-users-unsubscr...@hc.apache.org For additional commands, e-mail: httpclient-users-h...@hc.apache.org
