Possible memory leak in PushNotificationManager.pushedNotifications

[mwkirk]

Original author: beth...@gmail.com (May 02, 2012 16:37:55)

What steps will reproduce the problem?

1. Using the default enhanced mode.
2. Send a lot of pushes with no errors.
3. Since there are no errors, when processedFailedNotifications is called, ResponsePacketReader.processResponses never has anything to do and thus pushedNotifications.clear() is never called.

What is the expected output? What do you see instead? I expect it to not use 128 MB of RAM.

What version of the product are you using? On what operating system? 2.2.

Please provide any additional information below. Since you want to link the replies to the request, you'll need some way to timeout entries in pushedNotifications if no errors were reported after a few minutes.

It's possible I'm missing something, but I can't see how pushedNotifications gets cleared.

Original issue: http://code.google.com/p/javapns/issues/detail?id=120

[mwkirk]

From beth...@gmail.com on May 02, 2012 18:39:25 I added a breakpoint on the call to pushedNotifications.clear() in PushNotificationManager. It never gets hit.

This is running in thread mode.

Maybe PushNotificationManager.initializeConnection should do this?

this.pushedNotifications = new LinkedHashMap<Integer, PushedNotification>();

[mwkirk]

From sype...@gmail.com on May 02, 2012 21:17:53 Please refer to discussion in issue #88 to see if the explanations given there can help understand what you are seeing...

[mwkirk]

From beth...@gmail.com on May 03, 2012 01:00:45 I did see that. I assumed all changes from 2.2b1 would be in 2.2 final. If so, the issue isn't fixed for me. The LinkedHashMap<Integer, PushedNotification> is never cleared, because ResponsePacketReader.processResponses(this) never returns a non-zero result. That is the only place this map is cleared, it it's never hit. Running 2.2 I've had several out of memory errors. Is it fixed in the 2.3 trunk, then? I can try that, I guess.

[mwkirk]

From amir...@gmail.com on June 25, 2012 10:54:00 This is indeed a bug causing a memory leak.

The PushNotificationManager::sendNotification has the following line: if (!pushedNotifications.containsKey(notification.getIdentifier())) pushedNotifications.put(notification.getIdentifier(), notification);

The map is only cleared in processedFailedNotifications() which is called on stopConnection().

So if you leave the connection open and send multiple sendNotification requests, there will be a memory leak. This is not fixed in the 2.3 trunk yet (from what I could tell).

[mwkirk]

From sla...@hanmail.net on July 02, 2012 06:03:11 I tried to send notifications to over 50,000 users, and I've got an exception with follow messages.

Servlet.service() for servlet jsp threw exception java.lang.OutOfMemoryError: Java heap space at javapns.devices.Devices.asDevices(Devices.java:22) at javapns.Push.payload(Push.java:208) ...

I guess it is because of the same problem.

[mwkirk]

From sype...@gmail.com on July 02, 2012 21:51:53 Regarding the memory leak, improvements in that area should be included in an upcoming revision.

As for "slarin"'s issue, how much memory do you have allocated to your Java program? Pushing to 50,000 tokens would obviously require a bit of memory...

[mwkirk]

From ki...@crispygam.es on October 25, 2012 10:28:05 I think I am having the same issue. My heapdump shows about 32768 entries in the map pushedNotifications.

[mwkirk]

From ignavar...@gmail.com on November 09, 2012 14:03:07 I have the same issue. I have attached an image from JProfiler where you can see 2,420,179 instances of PushedNotification class. ( all of the stored in pushedNotifications variable from PushNotificationManager class ). The adopted workaround has been add the following line:

pushedNotifications.clear();

in the stopConnection method:

public void stopConnection() throws CommunicationException, KeystoreException { processedFailedNotifications(); pushedNotifications.clear(); try { logger.debug("Closing connection"); this.socket.close(); } catch (Exception e) { /* Do not complain if connection is already closed... */ } }

[mwkirk]

From b...@benxiong.org on November 13, 2012 12:34:21 if i reset pushQueue very 5 minutes, will there be connection or memory leak issue? Like this: every 5 minutes: pushQueue = Push.queue(certificationPath, certificationPassword, false, 2);

[mwkirk]

From m...@gogii.net on December 19, 2012 22:44:16 I too have run into memory issues. I've had a degree of success solving them without updating the javapns. I'm running 2.2 in QUEUE mode.

I've taken to logging and clearing the pushD queue after each time i queue a message.

I've wrapped the JnpsPushQueueBuilder to add JMX hooks for everything.

I've extended the PushNotificationManagerPushedNotifications to expose its package-protected internal queue accessor.

With respect to the connection cleanup issues, I'm going to test the latest and make edits to put back here. Has anyone had luck with this yet?

thanks!

matt

[mwkirk]

From giova...@brainweb.com.br on January 16, 2013 19:48:04 I have solved the problem. In the following methods:

public static PushedNotifications payload(Payload payload, Object keystore, String password, boolean production, int numberOfThreads, Object devices)

and

public static PushedNotifications payloads(Object keystore, String password, boolean production, int numberOfThreads, Object payloadDevicePairs)

change: return threads.getPushedNotifications(true);

to: PushedNotifications p = threads.getPushedNotifications(true); threads.destroy(); return p;

The memory leak was caused because of the use of the ThreadGroup.