From 223507f36f6dd4c0d4a0733a524fd529231b010b Mon Sep 17 00:00:00 2001 From: Nathan Fritz Date: Wed, 12 May 2010 13:45:36 -0700 Subject: [PATCH 1/2] fixed a rather large memory leak --- conn_tests/test_pubsubserver.py | 37 +++++++++++++++++++++++++++++++ sleekxmpp/plugins/xep_0045.py | 4 ++-- sleekxmpp/stanza/error.py | 2 +- sleekxmpp/stanza/htmlim.py | 3 ++- sleekxmpp/stanza/nick.py | 3 ++- sleekxmpp/xmlstream/stanzabase.py | 14 +++++++----- 6 files changed, 53 insertions(+), 10 deletions(-) diff --git a/conn_tests/test_pubsubserver.py b/conn_tests/test_pubsubserver.py index 7e5b57b..d1e2208 100644 --- a/conn_tests/test_pubsubserver.py +++ b/conn_tests/test_pubsubserver.py @@ -43,6 +43,10 @@ class TestPubsubServer(unittest.TestCase): def test001getdefaultconfig(self): """Get the default node config""" + self.xmpp1['xep_0060'].deleteNode(self.pshost, 'testnode2') + self.xmpp1['xep_0060'].deleteNode(self.pshost, 'testnode3') + self.xmpp1['xep_0060'].deleteNode(self.pshost, 'testnode4') + self.xmpp1['xep_0060'].deleteNode(self.pshost, 'testnode5') result = self.xmpp1['xep_0060'].getNodeConfig(self.pshost) self.statev['defaultconfig'] = result self.failUnless(isinstance(result, sleekxmpp.plugins.xep_0004.Form)) @@ -130,6 +134,39 @@ class TestPubsubServer(unittest.TestCase): self.failUnless(msg != False, "Account #1 did not get message event: perhaps node was advertised incorrectly?") self.failUnless(result) +# def test016speedtest(self): +# "Uncached speed test" +# import time +# start = time.time() +# for y in range(0, 50000, 1000): +# start2 = time.time() +# for x in range(y, y+1000): +# self.failUnless(self.xmpp1['xep_0060'].subscribe(self.pshost, "testnode4", subscribee="testuser%s@whatever" % x)) +# print time.time() - start2 +# seconds = time.time() - start +# print "--", seconds +# print "---------" +# time.sleep(15) +# self.failUnless(self.xmpp1['xep_0060'].deleteNode(self.pshost, 'testnode4'), "Could not delete non-cached test node") + +# def test015speedtest(self): +# "cached speed test" +# result = self.xmpp1['xep_0060'].getNodeConfig(self.pshost) +# self.statev['defaultconfig'] = result +# self.statev['defaultconfig'].field['pubsub#node_type'].setValue("leaf") +# self.statev['defaultconfig'].field['sleek#saveonchange'].setValue(True) +# self.failUnless(self.xmpp1['xep_0060'].create_node(self.pshost, 'testnode4', self.statev['defaultconfig'])) +# self.statev['defaultconfig'].field['sleek#saveonchange'].setValue(False) +# self.failUnless(self.xmpp1['xep_0060'].create_node(self.pshost, 'testnode5', self.statev['defaultconfig'])) +# start = time.time() +# for y in range(0, 50000, 1000): +# start2 = time.time() +# for x in range(y, y+1000): +# self.failUnless(self.xmpp1['xep_0060'].subscribe(self.pshost, "testnode5", subscribee="testuser%s@whatever" % x)) +# print time.time() - start2 +# seconds = time.time() - start +# print "--", seconds + def test900cleanup(self): "Cleaning up" self.failUnless(self.xmpp1['xep_0060'].deleteNode(self.pshost, 'testnode2'), "Could not delete test node.") diff --git a/sleekxmpp/plugins/xep_0045.py b/sleekxmpp/plugins/xep_0045.py index 240e6b9..937c6f9 100644 --- a/sleekxmpp/plugins/xep_0045.py +++ b/sleekxmpp/plugins/xep_0045.py @@ -93,10 +93,10 @@ class MUCPresence(ElementBase): return self def getNick(self): - return self.parent['from'].resource + return self.parent()['from'].resource def getRoom(self): - return self.parent['from'].bare + return self.parent()['from'].bare def setNick(self, value): logging.warning("Cannot set nick through mucpresence plugin.") diff --git a/sleekxmpp/stanza/error.py b/sleekxmpp/stanza/error.py index a1454d1..15af662 100644 --- a/sleekxmpp/stanza/error.py +++ b/sleekxmpp/stanza/error.py @@ -22,7 +22,7 @@ class Error(ElementBase): self['type'] = 'cancel' self['condition'] = 'feature-not-implemented' if self.parent is not None: - self.parent['type'] = 'error' + self.parent()['type'] = 'error' def getCondition(self): for child in self.xml.getchildren(): diff --git a/sleekxmpp/stanza/htmlim.py b/sleekxmpp/stanza/htmlim.py index f9ee985..60686e4 100644 --- a/sleekxmpp/stanza/htmlim.py +++ b/sleekxmpp/stanza/htmlim.py @@ -31,4 +31,5 @@ class HTMLIM(ElementBase): return html def delHtml(self): - return self.__del__() + if self.parent is not None: + self.parent().xml.remove(self.xml) diff --git a/sleekxmpp/stanza/nick.py b/sleekxmpp/stanza/nick.py index 92a523d..ac7e360 100644 --- a/sleekxmpp/stanza/nick.py +++ b/sleekxmpp/stanza/nick.py @@ -22,4 +22,5 @@ class Nick(ElementBase): return self.xml.text def delNick(self): - return self.__del__() + if self.parent is not None: + self.parent().xml.remove(self.xml) diff --git a/sleekxmpp/xmlstream/stanzabase.py b/sleekxmpp/xmlstream/stanzabase.py index 57c727a..018e81c 100644 --- a/sleekxmpp/xmlstream/stanzabase.py +++ b/sleekxmpp/xmlstream/stanzabase.py @@ -9,6 +9,7 @@ from xml.etree import cElementTree as ET import logging import traceback import sys +import weakref if sys.version_info < (3,0): from . import tostring26 as tostring @@ -51,7 +52,10 @@ class ElementBase(tostring.ToString): subitem = None def __init__(self, xml=None, parent=None): - self.parent = parent + if parent is None: + self.parent = None + else: + self.parent = weakref.ref(parent) self.xml = xml self.plugins = {} self.iterables = [] @@ -158,7 +162,7 @@ class ElementBase(tostring.ToString): else: self.xml.append(new) if self.parent is not None: - self.parent.xml.append(self.xml) + self.parent().xml.append(self.xml) return True #had to generate XML else: return False @@ -302,9 +306,9 @@ class ElementBase(tostring.ToString): self.xml.append(xml) return self - def __del__(self): - if self.parent is not None: - self.parent.xml.remove(self.xml) + #def __del__(self): #prevents garbage collection of reference cycle + # if self.parent is not None: + # self.parent.xml.remove(self.xml) class StanzaBase(ElementBase): name = 'stanza' From ae41c08fecfe627627f2a4d9b3861d4ae24d673e Mon Sep 17 00:00:00 2001 From: Nathan Fritz Date: Wed, 12 May 2010 18:07:20 -0700 Subject: [PATCH 2/2] added test for unsolicided unavailable presence and fixed bug to make it pass --- sleekxmpp/basexmpp.py | 7 ++++--- tests/test_presencestanzas.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/sleekxmpp/basexmpp.py b/sleekxmpp/basexmpp.py index fef8538..13fe210 100644 --- a/sleekxmpp/basexmpp.py +++ b/sleekxmpp/basexmpp.py @@ -271,12 +271,13 @@ class basexmpp(object): name = self.roster[jid].get('name', '') if show == 'unavailable': logging.debug("%s %s got offline" % (jid, resource)) - if len(self.roster[jid]['presence']): - del self.roster[jid]['presence'][resource] - else: + del self.roster[jid]['presence'][resource] + if len(self.roster[jid]['presence']) == 0 and not self.roster[jid]['in_roster']: del self.roster[jid] if not wasoffline: self.event("got_offline", presence) + else: + return False self.event("changed_status", presence) name = '' if name: diff --git a/tests/test_presencestanzas.py b/tests/test_presencestanzas.py index 430c71c..23eb911 100644 --- a/tests/test_presencestanzas.py +++ b/tests/test_presencestanzas.py @@ -11,5 +11,21 @@ class testpresencestanzas(unittest.TestCase): p = self.p.Presence() p['type'] = 'dnd' self.failUnless(str(p) == "dnd") + + def testPresenceUnsolicitedOffline(self): + "Unsolicted offline presence does not spawn changed_status or update roster" + p = self.p.Presence() + p['type'] = 'unavailable' + p['from'] = 'bill@chadmore.com/gmail15af' + import sleekxmpp + c = sleekxmpp.ClientXMPP('crap@wherever', 'password') + happened = [] + def handlechangedpresence(event): + happened.append(True) + c.add_event_handler("changed_status", handlechangedpresence) + c._handlePresence(p) + self.failUnless(happened == [], "changed_status event triggered for superfulous unavailable presence") + self.failUnless(c.roster == {}, "Roster updated for superfulous unavailable presence") + suite = unittest.TestLoader().loadTestsFromTestCase(testpresencestanzas)