Bug 59904 - memory leak--ServerCookie
Summary: memory leak--ServerCookie
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 9.0.0.M9
Hardware: All All
: P2 major (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
: 59926 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-07-27 06:31 UTC by sunqi
Modified: 2016-08-15 21:54 UTC (History)
3 users (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description sunqi 2016-07-27 06:31:47 UTC
heap dump:

Class Name	                        Objects	 Shallow Heap	Retained Heap
org.apache.tomcat.util.buf.ByteChunk	13220789 634597872	>=643312600
org.apache.tomcat.util.buf.MessageBytes 13194914 633355872	>=1801383536
org.apache.tomcat.util.buf.CharChunk	13204864 528194560	>=569456560
org.apache.tomcat.util.http.ServerCookie 2579614 123821472	>=1879787624


million+ ServerCookie objects in the heap,it cause memery leak.

org.apache.coyote.Request objcet contain a ServerCookies,it is a ServerCookie array,and the array length is 2048. 


so i have a test



		GetMethod getMethod = new GetMethod("http://127.0.0.1:8080/");

		Header h = new Header();
		h.setName("Cookie");
		StringBuilder sb=new StringBuilder();
		for(int i=0;i<2000;i++){
			sb.append("a=b;");
		}
		h.setValue(sb.toString());
		getMethod.addRequestHeader(h);

		int statusCode = httpClient.executeMethod(getMethod);


debug and get two thousand cookies in the request,and ServerCookie arrays is cached in heap.

we hava maxHeaderCount and maxHttpHeaderSize,but Cookie just as one header
so we need maxCookieCount too.
Comment 1 sunqi 2016-07-27 11:25:32 UTC
ps

we use async servlet,(If using Servlet 3.0 asynchronous processing, a good default is to use the larger of maxThreads and the maximum number of expected concurrent requests (synchronous and asynchronous),it is much larger than the default value of 200.
Comment 2 Huxing Zhang 2016-07-27 14:43:07 UTC
We saw two issues in this case:

1) the size of org.apache.tomcat.util.http.Cookies#scookies array never gets shrunk after recycle,  once it is dynamically resized. 

2) the number of org.apache.tomcat.util.http.ServerCookie object can be dramatically large, which might lead to memory leak. A maxHeaderSize of 8k has limit the number of ServerCookie object to no more than 2k, assuming each cookie is 4 bytes (e.g. 'a=b;'). This may have limited impact for a web application with low concurrency. However, for a heavily concurrent, NIO-based connector, this may have a  huge impact. Suppose a malicious client is sending 2k concurrent request, each sending 2k cookies, at tomcat side there will be 2k * 2k = 4m ServerCookie objects. Since the default maxConnection value is 10k for NIO, there can be at most 20m Server Cookies objects, and cannot be recycled because of 1).


Proposal:

1) We propose to shrink the org.apache.tomcat.util.http.ServerCookie object array back to its initial size after recycling org.apache.tomcat.util.http.Cookies object.

2) We propose to add a maxCookieCount configuration to limit the number of cookie to process. When exceeding the limit, extra data are ignored and a warning message is logged. The default value of maxCookieCount shall be unlimited to be compatible with current behavior, and configurable via server.xml.

We are currently working on a patch of 1) and 2).
Comment 3 Huxing Zhang 2016-07-28 05:05:31 UTC
> A maxHeaderSize of 8k has limit the number of ServerCookie object to no more than 2k

Correction:

We have just observed many 4k ServerCookie object array in org.apache.tomcat.util.http.Cookies#scookies in our latest heap dump.

Since spec [1] does not require cooke name and cookie value to be non-empty, given maxHeaderSize=8k, the max number of ServerCookie object in a request will be 4k, if if cookie string is '=;=;=;=;=;...'.

We argue the growing strategy here that the number of ServerCookie object should have an upper limit, which is maxHeaderSize / 2.

For example, if maxHeaderSize is 5k, then the number of ServerCookie object should be no more than 2.5k, hence the growing from 2k -> 4k will be unnecessary.


[1] https://tools.ietf.org/html/rfc6265#section-4.2.2
Comment 4 Remy Maucherat 2016-08-02 14:44:51 UTC
*** Bug 59911 has been marked as a duplicate of this bug. ***
Comment 5 Remy Maucherat 2016-08-02 14:45:44 UTC
59911 has a patch idea.
Comment 6 Remy Maucherat 2016-08-02 14:47:18 UTC
*** Bug 59926 has been marked as a duplicate of this bug. ***
Comment 7 Huxing Zhang 2016-08-03 08:57:49 UTC
The proposed patch in 59926 is against tomcat 9 trunk.
Comment 8 Mark Thomas 2016-08-11 11:05:39 UTC
I did some testing using:
http://browsercookielimits.squawky.net/

Focusing on max cookies per domain, the results are:
FireFox + Win = 150
FireFox + OSX = 150
Chrome + Win  = 180
Chrome + OSX  = 180
IE 11 + Win   =  50
Safari + OSX  = unlimited?

Therefore, I think enforcing a default limit of 200 is reasonable. That should make the patch simpler since I don't see the need for tracking the attainableMaxCookieCount with this default.
Comment 9 Mark Thomas 2016-08-11 19:50:09 UTC
This has been fixed in the following branches:
- 9.0.x for 9.0.0.M10 onwards
- 8.5.x for 8.5.5 onwards
- 8.0.x for 8.0.37 onwards
- 7.0.x for 7.0.71 onwards
- 6.0.x for 6.0.46 onwards

Thanks for the patch suggestion. It was used, and back-ported, with a few minor changes.
Comment 10 Christopher Schultz 2016-08-15 21:54:35 UTC
Does the cookies-per-domain increase if a subdomain is being used? For example, will the browser separately limit cookies for example.com to 150 but also foo.example.com to another 150, so that foo.example.com could potentially get 300 cookies?