spring-cloud-netflix: may be found a bug for httpclient connection leak

@brenuart The codes you provided works fine. Fixed my issue. Thanks. @zhangxsh Thanks for your explanation and guide. It helps me a lot.

@hooverhe no idea… But if you want to make sure connections and other resources are always disposed after the request is processed and can’t wait for this issue to be fixed, you can add the following to your Zuul application:

1/ Create a custom RequestContext implementation as follows:

public class GH2831RequestContext extends RequestContext {
	@Override
	public void unset() {
		disposeResources();
		super.unset();
	}
	
	/**
	 * Dispose {@code zuulResponse} and {@code responseDataStream} when the RequestContext is 
	 * unset (ie at request completion).
	 */
	private void disposeResources() {
		disposeResource(this.get("zuulResponse"));
		disposeResource(this.getResponseDataStream());
	}
	
	
	private void disposeResource(Object resource) {
		if (resource instanceof Closeable) {
			disposeResource((Closeable)resource);
		}
	}
	
	private void disposeResource(Closeable closeable) {
		if (closeable!=null) {
    		try {
    			closeable.close();
    		}
    		catch(Exception e) {
    			ReflectionUtils.rethrowRuntimeException(e);
    		}
	}
}

2/ Add the following in your @Configuration class:

static {
	RequestContext.setContextClass(GH2831RequestContext.class);
}

That should help… 😉

My feeling is the issue is larger than what originally described. In short: there is no guarantee the upstream response is always closed.

For instance, although SendResponseFilter#writeResponse() does it’s best to guarantee that the response stream from origin is properly closed in all cases (cfr. release pooled connection). However, it does not cover exceptions thrown outside of thath method, like during addResponseHeader() for instance. A first solution could be to wrap the entire run() method with a try/finally and move the close logic there.

But still, this would only guarantee that the response is closed IF it goes through the SendResponseFilter. What would happen if an exception is raised somewhere between, say, the RibbonRoutingFilter and SendResponseFilter filters? Zuul should normally direct the request to the “error” filters - but none of them seems to care about closing the response like what the SendResponseFilter does.

A more robust approach would be:

• filters should explicitly register their resources needing to be disposed when the request is fully completed
• the RequestContext is unset() by the ZuulServletFilter at request completion. We could use a custom version that would dispose registered resources when unset() is invoked.

What you think ?

@hooverhe @brenuart bcz the expire close timer only close the connections which are expired but leak,leak means the connection can not to close or reuse.

bcz： 1、if exception throws，then the SendResponseFilter can not be invoked,and the connection are not to close:

finally {
			/**
			* Closing the wrapping InputStream itself has no effect on closing the underlying tcp connection since it's a wrapped stream. I guess for http
			* keep-alive. When closing the wrapping stream it tries to reach the end of the current request, which is impossible for infinite http streams. So
			* instead of closing the InputStream we close the HTTP response.
			*
			* @author Johannes Edmeier
			*/
			try {
				Object zuulResponse = RequestContext.getCurrentContext()
						.get("zuulResponse");
				if (zuulResponse instanceof Closeable) {
					((Closeable) zuulResponse).close();
				}
				outStream.flush();
				// The container will close the stream for us
			}
			catch (IOException ex) {
			log.warn("Error while sending response to client: " + ex.getMessage());
			}
		}

To see the stack trace,the close activity is doing by the method of “releaseConnection” in ConnectionHolder.java ，close means:

a、really close;
b、not really close ,only mark the connection reusable 

-------------------------------------split-----------------------------------------

private void releaseConnection(final boolean reusable) {
        if (this.released.compareAndSet(false, true)) {
            synchronized (this.managedConn) {
                if (reusable) {
                    this.manager.releaseConnection(this.managedConn,
                            this.state, this.validDuration, this.tunit);
                } else {
                    try {
                        this.managedConn.close();
                        log.debug("Connection discarded");
                    } catch (final IOException ex) {
                        if (this.log.isDebugEnabled()) {
                            this.log.debug(ex.getMessage(), ex);
                        }
                    } finally {
                        this.manager.releaseConnection(
                                this.managedConn, null, 0, TimeUnit.MILLISECONDS);
                    }
                }
            }
        }
    }

if ConnectionHolder not close the connection,the the connection can not reuse and can not cleared by the timer(bcz the timer will find the connection is active although it is idle in fact).

at last,the connection is terminated by the nginx(bcz of keepAlive_timeout config) unilaterally.

when nginx send a FIN package to zuul,then the tcp protocol return ACK and changes the status of this tcp to CLOSE_WAIT locally and notify the http client to close the socket,but the application do not response(no thread to use the socket,the connection is idle in fact),then the connection remains in the status of CLOSE_WAIT。 as the pic shows:

then the connection remains in CLOSE_WAIT . As we mentioned over that,more and more connections are in close_wait.the number of close_wait equals to the value of “max-per-route-connections”