Atmosphere / atmosphere

Event Driven WebSockets Framework with Cross-Browser Fallbacks
http://async-io.org/
3.69k stars 750 forks source link

Critical memory leak #748

Closed averri closed 11 years ago

averri commented 11 years ago

I would like to report a memory leak with Atmosphere.

I'm using Atmosphere with Primefaces, version 1.1.0.beta1 (the problem occurs with 1.0.4 too).

After some minutes, the JVM will get out of memory with the following code:

import org.primefaces.push.PushContext;
import org.primefaces.push.PushContextFactory;
import org.springframework.context.annotation.Scope;

import javax.annotation.PostConstruct;
import javax.annotation.PreDestroy;
import javax.inject.Named;
import java.util.Timer;
import java.util.TimerTask;

@Named
@Scope("application")
public class Publisher {

    private Timer ordersTimer;

    private StringBuilder sb = new StringBuilder();

    public Publisher() {
        ordersTimer = new Timer("orders-timer", false);
        for (int i = 0; i < 100000; i++) {
            sb.append("0123456789");
        }
    }

    @PreDestroy
    public void destroy() {
        ordersTimer.cancel();
    }

    @PostConstruct
    public void init() {

        final PushContext ctx = PushContextFactory.getDefault().getPushContext();

        ordersTimer.scheduleAtFixedRate(new TimerTask() {
            @Override
            public void run() {
                try {
                    ctx.push("aChannel", sb.toString());

                } catch (Exception e) {
                    System.err.println("Error on publishing.");
                }
            }
        }, 0L, 1000L);

    }

}

I've configured Atmosphere as:

<init-param>
            <param-name>org.atmosphere.cpr.broadcasterClass</param-name>
            <param-value>org.atmosphere.util.SimpleBroadcaster</param-value>
        </init-param>
        <init-param>
            <param-name>org.atmosphere.cpr.broadcaster.shareableThreadPool</param-name>
            <param-value>true</param-value>
        </init-param>
        <init-param>
            <param-name>org.atmosphere.cpr.broadcasterLifeCyclePolicy</param-name>
            <param-value>IDLE_DESTROY</param-value>
        </init-param>
        <init-param>
            <param-name>org.atmosphere.cpr.AtmosphereInterceptor.disableDefaults</param-name>
            <param-value>true</param-value>
        </init-param>
smithh032772 commented 11 years ago

For more details, please see/click URL below (Issue 4950 in PrimeFaces Issue Tracker).

http://code.google.com/p/primefaces/issues/detail?id=4950

averri commented 11 years ago

Important details about my environment:

Dev/prod: apache-tomcat-7.0.32.

Development: Notebook Intel Core i5 4Gb, running Windows 7 64 bits. jdk1.6.0_37 32bits.

Production: Amazon EC2 with Extra large high CPU instance type, running Linux Redhat Enterprise 6.3, 64bit. jdk1.7.0_9 64bits.

The memory leak occurs in both environments.

jfarcand commented 11 years ago

Hum...are you sure the leak is coming from Atmosphere? Can you catch a heap dump?

averri commented 11 years ago

I see with VisualVM: the JVM old generation grows indefinitely with 'char[]' data. I'm sure!

jfarcand commented 11 years ago

There is no memory leak reported on Atmosphere so I suspect there is something in your application that creates those char. Try to find where those chars are created.

smithh032772 commented 11 years ago

@averri, who or what software is responsible for the 'char[]' data? Your app or Atmosphere? I know that I use java Character (or char data type) extensively in my app, but I don't ever use 'char[]' in my app.

Are you filling a 'char[]' buffer and then pushing the data/message via Atmosphere (PrimeFaces Push)?

averri commented 11 years ago

Btw, my application does not create any char array. It creates only a huge string and pushes it every second. Look at the code example I provided (the StringBuffer).

averri commented 11 years ago

If I comment out the line 'ctx.push("aChannel", sb.toString());' no memory leak happens.

smithh032772 commented 11 years ago

Very interesting. Would it be better to use StringBuilder instead of StringBuffer? and I guess there would be no memory leak, if you use String, right? :)

I find this interesting, because you almost alarmed me. I just made a note to start switching a lot of my String code to StringBuilder, but I see you're using StringBuffer instead. :)

averri commented 11 years ago

Smith, it doesn't matter. Even if I use a fixed big string instead of StringBuilder/StringBuffer, the problems occurs.

smithh032772 commented 11 years ago

I was going to ask you earlier while I was researching this for my own purposes, but what are the JVM options for your app?

averri commented 11 years ago

Important note: the char[] I've mentioned is derived from the big string my application creates. But pay attention to the code: the big string is created by the StringBuilder. The Atmosphere is caching this big string (char[]) internally for some reason.

averri commented 11 years ago

Smith, the JVM options is:

-server -Xms1g -Xmx1g -XX:MaxPermSize=512m -XX:+UseConcMarkSweepGC -XX:+UseParNewGC

averri commented 11 years ago

The Atmosphere is caching each message I've pushed. But I would like to Atmosphere to DO NOT cache messages when there's no client connected.

smithh032772 commented 11 years ago

Well, that sounds like a 'new' issue you should post. Is memory leak and caching strings the same thing? :)

the new issue/topic might get more attention and might get resolved as well.

I was just looking at java hotspot options, and I saw some jvm options for strings, and those are for performance tuning your jvm.

http://www.oracle.com/technetwork/java/javase/tech/vmoptions-jsp-140102.html#PerformanceTuning

averri commented 11 years ago

Smith, YES, caching string when it's not to cache IS a memory leak!

smithh032772 commented 11 years ago

Okay, it might be best for you to locate Atmosphere's source code, which is responsible for caching char[], and present that here in this topic.

smithh032772 commented 11 years ago

What container and version are you using? If I'm not mistaking, I think Atmosphere uses container's websocket implementation. I gave up trying to use Glassfish 3.1.2.2 with PrimeFaces Push (Atmosphere), so I went with TomEE 1.5.1 SNAPSHOT (tomcat 7.0.33 or latest release).

Maybe the container's websocket implementation is responsible for caching big strings. Just a thought.

averri commented 11 years ago

Smith, I'm using Tomcat 7.0.32. Tried with Tomcat 7.0.33 also, with no success.

smithh032772 commented 11 years ago

I just searched google for the following:

java websocket cache string

and saw some interesting results, like the one below. check it out. addToCache(), retrieveFromCache().

https://code.google.com/p/dontpush/source/browse/trunk/dontpush-root/dontpush-addon-ozonelayer/src/main/java/org/vaadin/dontpush/server/DontPushBroadcasterCache.java?r=143

I don't know if this is the cause though.

smithh032772 commented 11 years ago

You may also want to search google for the following and scroll through the results:

java websocket cache

or

atmosphere websocket cache

It may be caching your big strings, but I don't think the 'feature' is caching strings, it may be caching something else related to websockets implementation.

smithh032772 commented 11 years ago

That URL i shared above seems to be a separate API/library (dontpush), I guess they added some code to avoid from caching. you may take a look. it's interesting that dontpush added code to addToCache and retrieveFromCache.

jfarcand commented 11 years ago

Atmosphere doesn't cache anything, unless some BroadcasterCache are installed by default, which I doubt. Can you set the log to DEBUG and attach the result here?

averri commented 11 years ago

Maybe this have something to do with it: https://github.com/Atmosphere/atmosphere/issues/752

averri commented 11 years ago

Please, check the log:

17:34:05.951 DEBUG AtmosphereFramework.autoConfigureService Atmosphere's Service Annotation Not Supported. Please add https://github.com/rmuller/infomas-asl as dependencies or your own AnnotationProcessor to support @Service 17:34:05.958 INFO AtmosphereFramework.autoDetectAtmosphereHandlers Auto detecting atmosphere handlers /WEB-INF/classes/ 17:34:06.041 INFO AtmosphereFramework.autoDetectWebSocketHandler Auto detecting WebSocketHandler in /WEB-INF/classes/ 17:34:06.063 INFO AtmosphereFramework.autoDetectContainer Atmosphere is using async support: org.atmosphere.container.Tomcat7AsyncSupportWithWebSocket running under container: Apache Tomcat/7.0.32 17:34:06.064 INFO AtmosphereFramework.initWebSocket Installed WebSocketProtocol org.atmosphere.websocket.protocol.SimpleHttpProtocol 17:34:06.069 INFO AtmosphereFramework.configureAtmosphereInterceptor Installed Default AtmosphereInterceptor [Android Interceptor Support, SSE Interceptor Support, JSONP Interceptor Support]. Set org.atmosphere.cpr.AtmosphereInterceptor.disableDefaults in your xml to disable them. 17:34:06.069 WARN AtmosphereFramework.init No BroadcasterCache configured. Broadcasted message between client reconnection will be LOST. It is recommended to configure the HeaderBroadcasterCache. 17:34:06.070 WARN AtmosphereFramework.init Neither TrackMessageSizeInterceptor or TrackMessageSizeFilter are installed. atmosphere.js may receive glued and incomplete message. 17:34:06.072 INFO AtmosphereFramework.init HttpSession supported: false 17:34:06.072 INFO AtmosphereFramework.init Using BroadcasterFactory: org.atmosphere.cpr.DefaultBroadcasterFactory 17:34:06.072 INFO AtmosphereFramework.init Using WebSocketProcessor: org.atmosphere.websocket.DefaultWebSocketProcessor 17:34:06.072 INFO AtmosphereFramework.init Using Broadcaster: org.atmosphere.cpr.DefaultBroadcaster 17:34:06.085 INFO AtmosphereFramework.init Atmosphere Framework 1.0.4 started. 17:34:06.086 INFO AtmosphereFramework.interceptor Installed AtmosphereInterceptor Atmosphere LifeCycle. 17:34:06.090 DEBUG DefaultBroadcasterFactory.lookup Added Broadcaster /* . Factory size: 0 17:34:06.099 INFO AtmosphereFramework.addAtmosphereHandler Installed AtmosphereHandler org.primefaces.push.PrimeAtmosphereHandler mapped to context-path: /* 17:34:06.135 DEBUG DefaultWebSocketProcessor.open Atmosphere detected WebSocket: org.atmosphere.container.version.TomcatWebSocket 17:34:06.150 DEBUG DefaultBroadcasterFactory.lookup Added Broadcaster /book/WINZ12 . Factory size: 1 17:34:06.374 DEBUG AsynchronousProcessor.timedout Timing out the connection for request AtmosphereRequest{ contextPath=/system servletPath=/primepush pathInfo=/book/WINZ12 requestURI=/system/primepush/book/WINZ12 requestURL=http://localhost:8080/system/primepush/book/WINZ12 destroyable=false} 17:34:06.377 DEBUG DefaultBroadcasterFactory.remove Removing Broadcaster /book/WINZ12 factory size now 1 17:34:06.379 DEBUG DefaultBroadcaster.removeAtmosphereResource This Broadcaster has been destroyed and cannot be used /book/WINZ12 by invoking removeAtmosphereResource(AtmosphereResource r) 17:34:06.382 DEBUG DefaultBroadcaster.removeAtmosphereResource This Broadcaster has been destroyed and cannot be used /book/WINZ12 by invoking removeAtmosphereResource(AtmosphereResource r) 17:34:06.383 DEBUG DefaultBroadcaster.removeAtmosphereResource This Broadcaster has been destroyed and cannot be used /book/WINZ12 by invoking removeAtmosphereResource(AtmosphereResource r) Exception in thread "operator-timer" java.lang.OutOfMemoryError: Java heap space at java.util.Arrays.copyOf(Arrays.java:2882) at java.lang.AbstractStringBuilder.expandCapacity(AbstractStringBuilder.java:100) at java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:572) at java.lang.StringBuffer.append(StringBuffer.java:320) at org.primefaces.json.JSONObject.toString(JSONObject.java:1338) at org.primefaces.push.PushContextImpl.toJSON(PushContextImpl.java:121) at org.primefaces.push.PushContextImpl.push(PushContextImpl.java:37) at br.com.robovespa.web.jsf.WebsocketPublisher$3.run(WebsocketPublisher.java:123) at java.util.TimerThread.mainLoop(Timer.java:512) at java.util.TimerThread.run(Timer.java:462)

jfarcand commented 11 years ago

What's you VM setting? I think you can reproduce the OOM outside Atmosphere...replace the

ctx.push("aChannel", sb.toString());

with

  sb.toStriing()

to see if you still have the OOM. I suspect yes.

averri commented 11 years ago

My JVM setting are:

-server -Xms1g -Xmx1g -XX:MaxPermSize=512m -XX:+UseConcMarkSweepGC -XX:+UseParNewGC

I'll do the mentioned test above.

averri commented 11 years ago

jfarcand, for sure, replacing the line 'ctx.push("aChannel", sb.toString());' with 'sb.toString();' no memory leak occurs.

The huge string inside StringBuilder is allocated only once in heap. The problem occurs when pushing that string periodically to Atmosphere, with the configuration I provided. Small or big string, it doesn't matter, the OOM will occur. I've choosed a big string in order to speed up OOM.

jfarcand commented 11 years ago

OK, I don't see where, in Atmosphere, that kind of leak could happens. I will try to isolate a test case.

averri commented 11 years ago

jfarcand, ok! Please, try to do it. I see the Primefaces v3.4.2 code, but it only delegates to Atmosphere.

averri commented 11 years ago

jfarcand, a question: what happens if there is no client connected to any websocket? The behavior I expect in my application is to DO NOT cache any messages sent by server.

In my application, only the server pushes messages to connected clients, each second. I don't care if any of the messages get lost.

jfarcand commented 11 years ago

That the expected behaior, e.g by default no message are cached.

smithh032772 commented 11 years ago

@jfarcand please consider @averri atmosphere config in web.xml. I see the following, would this cause possibly cause this issue? If threadpool is shareable, I would assume a cache would be introduced, but I could be wrong.

    <init-param>
        <param-name>org.atmosphere.cpr.broadcaster.shareableThreadPool</param-name>
        <param-value>true</param-value>
    </init-param>
smithh032772 commented 11 years ago

does shareable threadpool atmosphere implementation include caching any/all msgs pushed?

smithh032772 commented 11 years ago

what is the best/appropriate use case for the following to be set in web.xml, AND when is worst case to use the following?

    <init-param>
        <param-name>org.atmosphere.cpr.broadcaster.shareableThreadPool</param-name>
        <param-value>true</param-value>
    </init-param>
jfarcand commented 11 years ago

Like I said, there is no cache in Atmosphere by default. I don't think shareable thread pool will change something to your issue as you only create one Broadcaster. If you can package a unit test (or a way to reproduce it), send it to jfarcand@apache.org

jfarcand commented 11 years ago

The bug is in PrimeFaces. Will follow there.

averri commented 11 years ago

I've tested with and without org.atmosphere.cpr.broadcaster.shareableThreadPool and the results was the same (OOM). Let's look at Primefaces code.

jfarcand commented 11 years ago

@averri: I've pushed the fix in PrimeFaces. Grab the latest snapshot and let me know how it goes.

smithh032772 commented 11 years ago

@jfarcand , if the fix is in PrimeFaces, what atmosphere-runtime (and compat) version do you recommend to use? 1.0.4? and then use 1.0.5, when released?

jfarcand commented 11 years ago

@ smithh032772 it is always good to use the latest version, so for now 1.0.4

smithh032772 commented 11 years ago

Okay, that's good to know, because PrimeFaces 3.4.1 or 3.4.2 upgraded to Atmosphere runtime 1.0.2, and I don't know if it is good to use PrimeFaces Push (atmosphere-runtime 1.0.2) with atmosphere-runtime 1.0.4 dependency in my app.

smithh032772 commented 11 years ago

@jfarcand , can you view latest comments on PrimeFaces issue 4950 in PrimeFaces issue tracker?

http://code.google.com/p/primefaces/issues/detail?id=4950

It sees as though the fix/commit is not available yet (for download).

jfarcand commented 11 years ago

Fix is here and should be available soon.

smithh032772 commented 11 years ago

@averri, i think you are using Tomcat (since most have difficulties getting PrimeFaces Push to work with Glassfish), and I wanted to let you know that I found the following in tomee.xml (since I'm using TomEE 1.5.1). I wonder if tomcat has the same enabled and I wonder if this is related to or causing this issue.

#

String cache configuration.

tomcat.util.buf.StringCache.byte.enabled=true

tomcat.util.buf.StringCache.char.enabled=true

tomcat.util.buf.StringCache.trainThreshold=500000

tomcat.util.buf.StringCache.cacheSize=5000

averri commented 11 years ago

Smith, the Tomcat string cache configuration does not fix this issue. I've configured as follow:

String cache configuration.

tomcat.util.buf.StringCache.byte.enabled=false tomcat.util.buf.StringCache.char.enabled=false

tomcat.util.buf.StringCache.trainThreshold=500000

tomcat.util.buf.StringCache.cacheSize=5000

smithh032772 commented 11 years ago

@averri, okay about tomcat string cache configuration. Did you test Jeanfrancois' fix that he pushed? PrimeFaces repository has been updated with latest build/JAR. I downloaded that today; the JAR was dated 12.09.2012.

I think you tested JeanFrancois' fix immediately after he made it available, and I think you said that that does not fix the problem. How did you test that (earlier)? Did you get that source and do a PrimeFaces build yourself, and then test against/with your app?

averri commented 11 years ago

Smith, the Jeanfrancois' fix did not solve the problem. The fixed code had another memory leak, but it has nothing to do with the OOM of this issue. I'm trying to debug the code.

averri commented 11 years ago

Hi, I see the 'org.primefaces.push.PushContextImpl$1' (this is the anonymous class BroadcasterListener) instances growing in each call to 'ctx.push(...)'. This is the reason of memory leak. When there is no client connected to the channel, each BroadcasterListener is never destroyed. As the BroadcasterListener keeps a reference to the message string so it never gets collected by JVM GC.

You can see it in action with VisualVM.

jfarcand commented 11 years ago

@averri This is exactly what I've fixed in PrimeFaces 3.5-SNAPSHOT (look at the link above).