-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix native memory leak in GzipHandler #1429
Conversation
@denvned Before we can accept this Pull Request, you will need to sign an Eclipse Contributor Agreement. This is a requirement of the Eclipse foundation and is mandatory. It is very easy to set up, and instructions can be found here. |
Calling See Issue #293 along with the JVM bugs There are plans to add a Deflater/Inflater pool to get around this issue (See Issue #300), so that GzipHandler, Jetty Client, and WebSocket's Permessage-Deflate all benefit from the reusing Deflater/Inflater objects, and minimize the memory leak effect. |
@joakime But the issues on bugs.java.com you linked says that calling the |
We've been down this path before, several testcases on multiple Deflater use alone have shown the bug is still present, even if using This is not a big problem for most Java applications, that only use 1 Deflater over their lifetime, but Jetty can use 1 for each HTTP/1.1 connection, and multiple for each WebSocket connection (that uses permessage-deflate) extension, potentially 10's of thousands of Deflater/Inflater instances, when it only needs a few hundred and reuse them. |
I'm not trying to get you to change this PR, its still useful, as its right thing to do for a Deflater. |
@joakime Are you saying that calling |
Looks like this leak shows itself only with asynchronous responses. Some relevant info: http://www.devguli.com/blog/eng/java-deflater-and-outofmemoryerror/ Signed-off-by: Denis Nedelyaev <denvned@gmail.com>
Signed ECA. |
Note that the GzipHandler is already using a Threadlocal "pool" of deflators |
Looks like this leak shows itself only with asynchronous responses. Some relevant info: http://www.devguli.com/blog/eng/java-deflater-and-outofmemoryerror/ Signed-off-by: Denis Nedelyaev <denvned@gmail.com>
Just to clarify the discussion here. The However #300 still applies to Websocket and it could use a similar strategy. |
Improve low resource solution for scheduling strategy. Replaced the dual scheduling strategy with a single re-implementation of EatWhatYouKill that can adapt to act as ProduceConsume, ExectureProduceConsume or ProduceExecuteConsume as need be. Squashed commit of the following: commit 25eeb32 Author: Greg Wilkins <gregw@webtide.com> Date: Sat Apr 1 09:08:49 2017 +1100 renamed variables commit 4f370d8 Merge: 8159c50 823cbe1 Author: Greg Wilkins <gregw@webtide.com> Date: Fri Mar 31 11:54:26 2017 +1100 Merge branch 'jetty-9.4.x' into jetty-9.4.x-ewyk commit 8159c50 Merge: 5805a92 daf61cd Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 30 17:28:00 2017 +1100 Merge remote-tracking branch 'origin/jetty-9.3.x' into jetty-9.4.x-ewyk commit daf61cd Author: Denis Nedelyaev <denvned@gmail.com> Date: Thu Mar 30 04:15:32 2017 +0300 Fix memory leak in GzipHandler (#1429) Looks like this leak shows itself only with asynchronous responses. Some relevant info: http://www.devguli.com/blog/eng/java-deflater-and-outofmemoryerror/ Signed-off-by: Denis Nedelyaev <denvned@gmail.com> commit 5805a92 Merge: cfabbd2 dc759db Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 30 17:12:38 2017 +1100 Merge branch 'jetty-9.4.x' into jetty-9.4.x-ewyk commit cfabbd2 Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 30 16:04:57 2017 +1100 minor cleanups commit c7aa64a Merge: bacf51a 18f17ac Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 30 14:58:37 2017 +1100 Merge branch 'jetty-9.4.x' into jetty-9.4.x-ewyk commit bacf51a Merge: 11ba4bc 2fafa1d Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 30 14:13:36 2017 +1100 Merge branch 'jetty-9.4.x' into jetty-9.4.x-ewyk commit 11ba4bc Merge: 69003d3 1a0b2df Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 30 13:48:09 2017 +1100 Merge branch 'jetty-9.4.x' into jetty-9.4.x-ewyk commit 69003d3 Merge: f89b08d a8ff18d Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 30 12:35:27 2017 +1100 Merge branch 'jetty-9.4.x' into jetty-9.4.x-ewyk commit f89b08d Merge: 7a87c8e 00b42ca Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 23 16:01:00 2017 +1100 Merge branch 'jetty-9.4.x' into jetty-9.4.x-ewyk commit 7a87c8e Merge: 1a92015 12dc169 Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 23 10:27:14 2017 +1100 Merge remote-tracking branch 'origin/jetty-9.4.x' into jetty-9.4.x-ewyk commit 1a92015 Author: Greg Wilkins <gregw@webtide.com> Date: Tue Mar 21 09:23:53 2017 +1100 better spruious wakeup handling and other simplifications commit c01a910 Merge: 0b2b9ea 67ec4b0 Author: Greg Wilkins <gregw@webtide.com> Date: Fri Mar 17 14:59:37 2017 +1100 Merge branch 'jetty-9.4.x' into jetty-9.4.x-ewyk commit 0b2b9ea Author: Greg Wilkins <gregw@webtide.com> Date: Fri Mar 17 14:52:49 2017 +1100 cleanup commit c1d92eb Author: Greg Wilkins <gregw@webtide.com> Date: Fri Mar 17 13:41:45 2017 +1100 Fixed push commit d2d6bc3 Author: Greg Wilkins <gregw@webtide.com> Date: Fri Mar 17 12:18:03 2017 +1100 minor cleanups commit c1a159b Merge: 01349ac 78f4712 Author: Greg Wilkins <gregw@webtide.com> Date: Fri Mar 17 09:30:44 2017 +1100 Merge branch 'jetty-9.4.x' into jetty-9.4.x-ewyk commit 01349ac Merge: 4dc1503 08f351b Author: Greg Wilkins <gregw@webtide.com> Date: Fri Mar 17 08:16:06 2017 +1100 Merge remote-tracking branch 'origin/jetty-9.4.x' into jetty-9.4.x-ewyk commit 4dc1503 Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 16 23:26:59 2017 +1100 work in progress commit 5d18c65 Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 16 22:05:03 2017 +1100 work in progress commit d52a09a Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 16 18:03:16 2017 +1100 work in progress commit c097db3 Author: Greg Wilkins <gregw@webtide.com> Date: Thu Mar 16 15:59:29 2017 +1100 Experiement enhancement to EatWhatYouKill ExecutionStrategy Use the existence of a pending producer threads to determine if low resources or not.
Looks like this leak shows itself only with asynchronous responses.
Some relevant info: http://www.devguli.com/blog/eng/java-deflater-and-outofmemoryerror/