It seems like there is a memory leak in the smack 3.2 (PacketReader.java) even in latest smack version i.e. 3.3.
So when connection breaks abruptly , then notifyConnectionError() method is getting called in PacketReader.java. In this method , on the very first line they are setting the done variable to true and then calling connection.shutdown(new Presence(Presence.Type.unavailable)) in XMPPConnection.java class.
In shutdown method in XMPPConnection.java , they are calling **packetReader.shutdown(); , **in whiich they are checking again done variable , which is already set as true and the block of code which clears some data structure is not getting called , leading to OOME.
We are getting OOME in InBandBytestreamManager.java class where a Map known as **managers **is failing. The code to clear this Map is written in block which is not getting called because of done variable.
You are basically saying that connectionClosed() is not called in case of a unclean disconnect. Correct?
connectionClosedOnError(Exception e) should have been called instead. I don’t think that the problem is the done variable. Managers should behave the same if connectionClosedOnError is called and this is not the case for InBandBytestreamManager ( http://fisheye.igniterealtime.org/browse/smack/trunk/source/org/jivesoftware/sma ckx/bytestreams/ibb/InBandBytestreamManager.java?r1=12965&r2=12965&u=3#to106 ), this is the main problem.
So the quick fix would be just adding the below lines in the current code of InBandBytestreamManager.java . Please confirm.
public void connectionClosedOnError(Exception e) {
** manager.disableService();**
** }**
Also looking into smack code I found that there was one more manager class known as - Socks5BytestreamManager.java , which also maintain a Map of connection and this class instances , which is also not getting cleaned on unclean disconnect. Please see like if we have to clean the Map there as well.
Yes, that would also have been my first intuitive try to fix this issue.
While a better approach may be to change ConnectionListener to an abstract class, that implements calling connectionClosed as default behavior of connectionClosedOnError(). But that would require an API change.
And what is your thought about Socks5BytestreamManager.java class that I mention above, because it also has the same issue , should I put the same line of code there as well…
And what is your thought about Socks5BytestreamManager.java class that I mention above, because it also has the same issue , should I put the same line of code there as well…