powered by Jive Software

IQRouter / IQResultListener memory leak + fix

In IQRouter, a mechanism is in use that lets you wait for the answer (result or error) of an IQ that you send out. This mechanism registers IQResultListener objects for each ‘pending’ request.

If no result or error is returned to a request for which an IQResultListener has been registered, the IQResultListener object remains in memory forever. This causes a memory leak.

There’s a TODO comment in the IQRouter class, that describes a possible fix:

TODO: Add a check that if no IQ reply was received for a while then an IQ error should be generated by the server and simulate like the client sent it. This will let listeners react and be removed from the collection

I don’t quite agree with the comment that is made in the code. The decision to generate and return an IQ-error to be on behalf of the client is based on a wrong assumption, and can cause unexpected behavior (indefinite stanza retransmission is likely to be caused by this, for example).

Assuming that, because of a lack of result, the client did not correctly parse the original stanza, is wrong. A likely cause of not receiving a response is a client bug that causes a client to simply fail to respond with <iq type=“result” /> to correctly parsed stanzas. Generating an XMPP error on behalf of these clients is wrong, in my opinion.

I suggest to simply remove the listener from the Map, and log a related warning instead. A new method could be added to the IQResultListener interface which would be called if no answer was received within a certain time frame ( #noAnswerReceived(String packetId). This would allow each specific implementation to handle the situation as it sees fit.

I’ve added an update to IQRouter in the attached file. It assumes that the IQResultListener has received the new method that I described. This in turn will trigger three code changes in the core Openfire code, which can be as simple as a call to Log.warn(). As an added benefit, the somewhat messy code in InternalComponentManager#query() (which currently explicitly waits for an answer) can be cleaned up.

Hey Guus,

Your changes have been incorporated. I filed the issue JM-1151 and checked in your changes.

Thanks,

– Gato