From e0c32b6d9bc357e79db04ea3126a614e038af928 Mon Sep 17 00:00:00 2001 From: Brian Beggs Date: Wed, 5 May 2010 02:03:38 +0800 Subject: [PATCH 1/3] Fixes for disconnection problems detailed in http://github.com/fritzy/SleekXMPP/issues/#issue/20 Fixes to both ClientXMPP & xmlstream. ClientXMPP was not tracking the changes to authenticated and sessionstarted after the client was disconnected. xmlstream had some funkyness with state in the _process method that was cleaned up and hopefully made a little cleaner. Also changed a DNS issue that was occuring that rendered me unable to disconnect. I would recieve the following error upon reconnect. Exception in thread process: Exception in thread process: Traceback (most recent call last): File "/usr/local/lib/python2.6/threading.py", line 532, in __bootstrap_inner self.run() File "/usr/local/lib/python2.6/threading.py", line 484, in run self.__target(*self.__args, **self.__kwargs) File "/home/macdiesel/tmp/workspace/SleekXMPP/sleekxmpp/xmlstream/xmlstream.py", line 202, in _process self.reconnect() File "/home/macdiesel/tmp/workspace/SleekXMPP/sleekxmpp/__init__.py", line 134, in reconnect XMLStream.reconnect(self) File "/home/macdiesel/tmp/workspace/SleekXMPP/sleekxmpp/xmlstream/xmlstream.py", line 289, in reconnect self.connect() File "/home/macdiesel/tmp/workspace/SleekXMPP/sleekxmpp/__init__.py", line 99, in connect answers = dns.resolver.query("_xmpp-client._tcp.%s" % self.server, "SRV") File "/usr/local/lib/python2.6/site-packages/dns/resolver.py", line 732, in query return get_default_resolver().query(qname, rdtype, rdclass, tcp, source) File "/usr/local/lib/python2.6/site-packages/dns/resolver.py", line 617, in query source=source) File "/usr/local/lib/python2.6/site-packages/dns/query.py", line 113, in udp wire = q.to_wire() File "/usr/local/lib/python2.6/site-packages/dns/message.py", line 404, in to_wire r.add_question(rrset.name, rrset.rdtype, rrset.rdclass) File "/usr/local/lib/python2.6/site-packages/dns/renderer.py", line 152, in add_question self.output.write(struct.pack("!HH", rdtype, rdclass)) TypeError: unsupported operand type(s) for &: 'unicode' and 'long' Seems I was getting this error when calling line 99 in ClientXMPP. You can't bit-shift a 1 and a string and this is why this error is coming up. I removed the "SRV" argument and used the default of 1. not sure exactly what this should be so it may need to be fixed back before it's merged back to trunk. The line in question: answers = dns.resolver.query("_xmpp-client._tcp.%s" % self.server, "SRV") --- .gitignore | 1 + sleekxmpp/__init__.py | 6 +++++- sleekxmpp/xmlstream/xmlstream.py | 6 ++++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 0fe2c40..788de4c 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ *.pyc +.project build/ diff --git a/sleekxmpp/__init__.py b/sleekxmpp/__init__.py index 773f793..e2cfb1b 100644 --- a/sleekxmpp/__init__.py +++ b/sleekxmpp/__init__.py @@ -96,7 +96,7 @@ class ClientXMPP(basexmpp, XMLStream): else: logging.debug("Since no address is supplied, attempting SRV lookup.") try: - answers = dns.resolver.query("_xmpp-client._tcp.%s" % self.server, "SRV") + answers = dns.resolver.query("_xmpp-client._tcp.%s" % self.server) except dns.resolver.NXDOMAIN: logging.debug("No appropriate SRV record found. Using JID server name.") else: @@ -131,10 +131,14 @@ class ClientXMPP(basexmpp, XMLStream): def reconnect(self): logging.info("Reconnecting") self.event("disconnected") + self.authenticated = False + self.sessionstarted = False XMLStream.reconnect(self) def disconnect(self, init=True, close=False, reconnect=False): self.event("disconnected") + self.authenticated = False + self.sessionstarted = False XMLStream.disconnect(self, reconnect) def registerFeature(self, mask, pointer, breaker = False): diff --git a/sleekxmpp/xmlstream/xmlstream.py b/sleekxmpp/xmlstream/xmlstream.py index 025884b..13a87a6 100644 --- a/sleekxmpp/xmlstream/xmlstream.py +++ b/sleekxmpp/xmlstream/xmlstream.py @@ -199,9 +199,11 @@ class XMLStream(object): traceback.print_exc() self.disconnect(reconnect=True) if self.state['reconnect']: + self.state.set('connected', False) + self.state.set('processing', False) self.reconnect() - self.state.set('processing', False) - self.eventqueue.put(('quit', None, None)) + else: + self.eventqueue.put(('quit', None, None)) #self.__thread['readXML'] = threading.Thread(name='readXML', target=self.__readXML) #self.__thread['readXML'].start() #self.__thread['spawnEvents'] = threading.Thread(name='spawnEvents', target=self.__spawnEvents) From 4c410dd48aefc69682c30f28e830e5dee62a04ab Mon Sep 17 00:00:00 2001 From: Nathan Fritz Date: Thu, 13 May 2010 04:45:36 +0800 Subject: [PATCH 2/3] 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 7522839141e7dd5bd081a421a58b0962b705fdda Mon Sep 17 00:00:00 2001 From: Nathan Fritz Date: Thu, 13 May 2010 09:07:20 +0800 Subject: [PATCH 3/3] 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)