Opened 4 years ago

Last modified 4 years ago

#4313 defect new

jabber.client.IQ memory-leaks observer objects

Reported by: alsuren Owned by:
Priority: low Milestone:
Component: words Keywords:
Cc: thijs Branch:
Author: Launchpad Bug:

Description

If no callbacks are registered, the IQ still registers an observer. While this is only in theory a small, temporary leak, there doesn't seem to be a way to specify a timeout, so a remote user can trick us into OOMing ourselves by eg failing to reply to jingle IQs (We found this when load testing http://telepathy.freedesktop.org/wiki/Fargo with lazy test scripts)

Also, when registering the observer, it passes in a string. This string gets turned into an xpath query object and interned. Because each iq's id is unique, this interned query is guaranteed to never be used again. Please construct a query object manually so that it doesn't get interned.

work-around:
Don't use IQ, and construct domish.Elements by hand instead. This is what we ended up doing in Fargo. We still need to add the appropriate code for relaying IQ errors to jingle call objects. Might implement it by keeping a WeakValueDict of id->channel or something to avoid leaks.

Change History (5)

comment:1 follow-up: Changed 4 years ago by exarkun

  • Summary changed from jabber.client.IQ memory-leaks like a dead GI's helmet. to jabber.client.IQ memory-leaks observer objects

comment:2 in reply to: ↑ 1 Changed 4 years ago by thijs

  • Cc thijs added

i liked the dead helmet!

comment:3 Changed 4 years ago by ralphm

I suppose t.w.p.j.client.IQ should just be deprecated. There has been a new implementation of this in t.w.p.j.xmlstream.IQ for a while now. On send it returns a deferred that is fired with the response stanza. It registers exactly one observer and keeps record of stanza identifiers and deferreds to fire. It has timeout and connection-lost support. I'm not yet entirely happy with its internals and how the stream is passed to it, but it is way way better.

comment:4 Changed 4 years ago by ralphm

  • Owner changed from exarkun to ralphm

comment:5 Changed 3 years ago by <automation>

  • Owner ralphm deleted
Note: See TracTickets for help on using tickets.