From 0aee445e6976b03f7e04bcab6ec40ecf14b5d1a4 Mon Sep 17 00:00:00 2001 From: Lance Stout Date: Thu, 16 Dec 2010 22:21:50 -0500 Subject: [PATCH 1/5] Use daemon threads instead of signals. Daemonized threads exit once the main program has quit, and the only threads left running are all daemon threads. Should fix hanging clients while not trampling over anyone else's signal handlers. --- sleekxmpp/xmlstream/xmlstream.py | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/sleekxmpp/xmlstream/xmlstream.py b/sleekxmpp/xmlstream/xmlstream.py index 9e91b5d..2317f04 100644 --- a/sleekxmpp/xmlstream/xmlstream.py +++ b/sleekxmpp/xmlstream/xmlstream.py @@ -16,7 +16,6 @@ import sys import threading import time import types -import signal try: import queue except ImportError: @@ -209,24 +208,6 @@ class XMLStream(object): self.auto_reconnect = True self.is_client = False - try: - if hasattr(signal, 'SIGHUP'): - signal.signal(signal.SIGHUP, self._handle_kill) - if hasattr(signal, 'SIGTERM'): - # Used in Windows - signal.signal(signal.SIGTERM, self._handle_kill) - except: - log.debug("Can not set interrupt signal handlers. " + \ - "SleekXMPP is not running from a main thread.") - - def _handle_kill(self, signum, frame): - """ - Capture kill event and disconnect cleanly after first - spawning the "killed" event. - """ - self.event("killed", direct=True) - self.disconnect() - def new_id(self): """ Generate and return a new stream ID in hexadecimal form. @@ -705,6 +686,7 @@ class XMLStream(object): def start_thread(name, target): self.__thread[name] = threading.Thread(name=name, target=target) + self.__thread[name].daemon = True self.__thread[name].start() for t in range(0, HANDLER_THREADS): From 53a5026301b6f6e842fa29b52bef1721e068eddf Mon Sep 17 00:00:00 2001 From: Lance Stout Date: Thu, 16 Dec 2010 23:52:17 -0500 Subject: [PATCH 2/5] Almost done with xep-30; added more docs. --- sleekxmpp/plugins/xep_0030/disco.py | 8 ++--- sleekxmpp/plugins/xep_0030/static.py | 47 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/sleekxmpp/plugins/xep_0030/disco.py b/sleekxmpp/plugins/xep_0030/disco.py index ad3d0ae..1b78d52 100644 --- a/sleekxmpp/plugins/xep_0030/disco.py +++ b/sleekxmpp/plugins/xep_0030/disco.py @@ -230,7 +230,7 @@ class xep_0030(base_plugin): no stanzas need to be sent. Otherwise, a disco stanza must be sent to the remove JID to retrieve the info. - dfrom -- Specifiy the sender's JID. + ifrom -- Specifiy the sender's JID. block -- If true, block and wait for the stanzas' reply. timeout -- The time in seconds to block while waiting for a reply. If None, then wait indefinitely. @@ -245,7 +245,7 @@ class xep_0030(base_plugin): return self._fix_default_info(info) iq = self.xmpp.Iq() - iq['from'] = kwargs.get('dfrom', '') + iq['from'] = kwargs.get('ifrom', '') iq['to'] = jid iq['type'] = 'get' iq['disco_info']['node'] = node if node else '' @@ -270,7 +270,7 @@ class xep_0030(base_plugin): no stanzas need to be sent. Otherwise, a disco stanza must be sent to the remove JID to retrieve the items. - dfrom -- Specifiy the sender's JID. + ifrom -- Specifiy the sender's JID. block -- If true, block and wait for the stanzas' reply. timeout -- The time in seconds to block while waiting for a reply. If None, then wait indefinitely. @@ -282,7 +282,7 @@ class xep_0030(base_plugin): return self._run_node_handler('get_items', jid, node, kwargs) iq = self.xmpp.Iq() - iq['from'] = kwargs.get('dfrom', '') + iq['from'] = kwargs.get('ifrom', '') iq['to'] = jid iq['type'] = 'get' iq['disco_items']['node'] = node if node else '' diff --git a/sleekxmpp/plugins/xep_0030/static.py b/sleekxmpp/plugins/xep_0030/static.py index b0e931b..11674e6 100644 --- a/sleekxmpp/plugins/xep_0030/static.py +++ b/sleekxmpp/plugins/xep_0030/static.py @@ -31,6 +31,11 @@ class StaticDisco(object): StaticDisco provides a set of node handlers that will store static sets of disco info and items in memory. + + Attributes: + nodes -- A dictionary mapping (JID, node) tuples to a dict + containing a disco#info and a disco#items stanza. + xmpp -- The main SleekXMPP object. """ def __init__(self, xmpp): @@ -47,6 +52,14 @@ class StaticDisco(object): self.xmpp = xmpp def add_node(self, jid=None, node=None): + """ + Create a new set of stanzas for the provided + JID and node combination. + + Arguments: + jid -- The JID that will own the new stanzas. + node -- The node that will own the new stanzas. + """ if jid is None: jid = self.xmpp.boundjid.full if node is None: @@ -57,7 +70,21 @@ class StaticDisco(object): self.nodes[(jid, node)]['info']['node'] = node self.nodes[(jid, node)]['items']['node'] = node + # ================================================================= + # Node Handlers + # + # Each handler accepts three arguments: jid, node, and data. + # The jid and node parameters together determine the set of + # info and items stanzas that will be retrieved or added. + # The data parameter is a dictionary with additional paramters + # that will be passed to other calls. + def get_info(self, jid, node, data): + """ + Return the stored info data for the requested JID/node combination. + + The data parameter is not used. + """ if (jid, node) not in self.nodes: if not node: return DiscoInfo() @@ -67,10 +94,20 @@ class StaticDisco(object): return self.nodes[(jid, node)]['info'] def del_info(self, jid, node, data): + """ + Reset the info stanza for a given JID/node combination. + + The data parameter is not used. + """ if (jid, node) in self.nodes: self.nodes[(jid, node)]['info'] = DiscoInfo() def get_items(self, jid, node, data): + """ + Return the stored items data for the requested JID/node combination. + + The data parameter is not used. + """ if (jid, node) not in self.nodes: if not node: return DiscoInfo() @@ -80,11 +117,21 @@ class StaticDisco(object): return self.nodes[(jid, node)]['items'] def set_items(self, jid, node, data): + """ + Replace the stored items data for a JID/node combination. + + The data parameter is not used. + """ items = data.get('items', set()) self.add_node(jid, node) self.nodes[(jid, node)]['items']['items'] = items def del_items(self, jid, node, data): + """ + Reset the items stanza for a given JID/node combination. + + The data parameter is not used. + """ if (jid, node) in self.nodes: self.nodes[(jid, node)]['items'] = DiscoItems() From 982bf3b2ecba86a76badf8cacbe82a8f5fc00b80 Mon Sep 17 00:00:00 2001 From: Florent Le Coz Date: Fri, 17 Dec 2010 23:33:13 +0800 Subject: [PATCH 3/5] RootStanza raises unexpected exceptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We now raise the unexpected exceptions instead of sending them on the network. - avoids flood (sending a traceback on a MUC, for example…) and maybe some security issues. - lets you handle the traceback (catch it to handle it properly, or with except_hook, etc) - an exception cannot be raised without you knowing --- sleekxmpp/stanza/rootstanza.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/sleekxmpp/stanza/rootstanza.py b/sleekxmpp/stanza/rootstanza.py index 6975c72..777314b 100644 --- a/sleekxmpp/stanza/rootstanza.py +++ b/sleekxmpp/stanza/rootstanza.py @@ -54,16 +54,17 @@ class RootStanza(StanzaBase): e.extension_args) self['error'].append(extxml) self['error']['type'] = e.etype + self.send() else: - # We probably didn't raise this on purpose, so send a traceback + # We probably didn't raise this on purpose, so send an error stanza self['error']['condition'] = 'undefined-condition' - if sys.version_info < (3, 0): - self['error']['text'] = "SleekXMPP got into trouble." - else: - self['error']['text'] = traceback.format_tb(e.__traceback__) - log.exception('Error handling {%s}%s stanza' % - (self.namespace, self.name)) - self.send() - + self['error']['text'] = "SleekXMPP got into trouble." + self.send() + # log the error + log.exception('Error handling {%s}%s stanza' % + (self.namespace, self.name)) + # Finally raise the exception, so it can be handled (or not) + # at a higher level + raise e register_stanza_plugin(RootStanza, Error) From 34c374a1e1dad468869c8e5dc6fb2ea7b1d90c7e Mon Sep 17 00:00:00 2001 From: Lance Stout Date: Fri, 17 Dec 2010 13:11:03 -0500 Subject: [PATCH 4/5] Make tests pass for catching exceptions. May now use sys.excepthook to catch exceptions from threaded handlers. --- sleekxmpp/stanza/rootstanza.py | 2 +- sleekxmpp/xmlstream/xmlstream.py | 24 +++++++++++++++++++++ tests/test_stream_exceptions.py | 37 +++++++++++++++++++------------- 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/sleekxmpp/stanza/rootstanza.py b/sleekxmpp/stanza/rootstanza.py index 777314b..8123c5f 100644 --- a/sleekxmpp/stanza/rootstanza.py +++ b/sleekxmpp/stanza/rootstanza.py @@ -64,7 +64,7 @@ class RootStanza(StanzaBase): log.exception('Error handling {%s}%s stanza' % (self.namespace, self.name)) # Finally raise the exception, so it can be handled (or not) - # at a higher level + # at a higher level by using sys.excepthook. raise e register_stanza_plugin(RootStanza, Error) diff --git a/sleekxmpp/xmlstream/xmlstream.py b/sleekxmpp/xmlstream/xmlstream.py index 2317f04..d5c1043 100644 --- a/sleekxmpp/xmlstream/xmlstream.py +++ b/sleekxmpp/xmlstream/xmlstream.py @@ -682,6 +682,7 @@ class XMLStream(object): Event handlers and the send queue will be threaded regardless of this parameter's value. """ + self._thread_excepthook() self.scheduler.process(threaded=True) def start_thread(name, target): @@ -954,3 +955,26 @@ class XMLStream(object): self.disconnect() self.event_queue.put(('quit', None, None)) return + + def _thread_excepthook(self): + """ + If a threaded event handler raises an exception, there is no way to + catch it except with an excepthook. Currently, each thread has its own + excepthook, but ideally we could use the main sys.excepthook. + + Modifies threading.Thread to use sys.excepthook when an exception + is not caught. + """ + init_old = threading.Thread.__init__ + def init(self, *args, **kwargs): + init_old(self, *args, **kwargs) + run_old = self.run + def run_with_except_hook(*args, **kw): + try: + run_old(*args, **kw) + except (KeyboardInterrupt, SystemExit): + raise + except: + sys.excepthook(*sys.exc_info()) + self.run = run_with_except_hook + threading.Thread.__init__ = init diff --git a/tests/test_stream_exceptions.py b/tests/test_stream_exceptions.py index b7be648..e1b70d3 100644 --- a/tests/test_stream_exceptions.py +++ b/tests/test_stream_exceptions.py @@ -10,6 +10,7 @@ class TestStreamExceptions(SleekTest): """ def tearDown(self): + sys.excepthook = sys.__excepthook__ self.stream_close() def testXMPPErrorException(self): @@ -78,9 +79,16 @@ class TestStreamExceptions(SleekTest): def testUnknownException(self): """Test raising an generic exception in a threaded handler.""" + raised_errors = [] + def message(msg): raise ValueError("Did something wrong") + def catch_error(*args, **kwargs): + raised_errors.append(True) + + sys.excepthook = catch_error + self.stream_start() self.xmpp.add_event_handler('message', message) @@ -90,21 +98,20 @@ class TestStreamExceptions(SleekTest): """) - if sys.version_info < (3, 0): - self.send(""" - - - - - SleekXMPP got into trouble. - - - - """) - else: - # Unfortunately, tracebacks do not make for very portable tests. - pass + self.send(""" + + + + + SleekXMPP got into trouble. + + + + """) + + self.assertEqual(raised_errors, [True], "Exception was not raised: %s" % raised_errors) + suite = unittest.TestLoader().loadTestsFromTestCase(TestStreamExceptions) From f97f6e5985b5781ce87a2095b4bf9cccb12ae978 Mon Sep 17 00:00:00 2001 From: Lance Stout Date: Tue, 21 Dec 2010 11:33:03 -0500 Subject: [PATCH 5/5] More documentation for XEP-0030 plugin. --- sleekxmpp/plugins/xep_0030/disco.py | 5 +- sleekxmpp/plugins/xep_0030/static.py | 79 ++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/sleekxmpp/plugins/xep_0030/disco.py b/sleekxmpp/plugins/xep_0030/disco.py index 1b78d52..92ee5ec 100644 --- a/sleekxmpp/plugins/xep_0030/disco.py +++ b/sleekxmpp/plugins/xep_0030/disco.py @@ -108,7 +108,8 @@ class xep_0030(base_plugin): self._disco_ops = ['get_info', 'set_identities', 'set_features', 'get_items', 'set_items', 'del_items', 'add_identity', 'del_identity', 'add_feature', - 'del_feature', 'add_item', 'del_item'] + 'del_feature', 'add_item', 'del_item', + 'del_identities', 'del_features'] self._handlers = {} for op in self._disco_ops: self._handlers[op] = {'global': getattr(self.static, op), @@ -141,8 +142,10 @@ class xep_0030(base_plugin): set_features set_items del_items + del_identities del_identity del_feature + del_features del_item add_identity add_feature diff --git a/sleekxmpp/plugins/xep_0030/static.py b/sleekxmpp/plugins/xep_0030/static.py index 11674e6..eff67f0 100644 --- a/sleekxmpp/plugins/xep_0030/static.py +++ b/sleekxmpp/plugins/xep_0030/static.py @@ -136,6 +136,15 @@ class StaticDisco(object): self.nodes[(jid, node)]['items'] = DiscoItems() def add_identity(self, jid, node, data): + """ + Add a new identity to te JID/node combination. + + The data parameter may provide: + category -- The general category to which the agent belongs. + itype -- A more specific designation with the category. + name -- Optional human readable name for this identity. + lang -- Optional standard xml:lang value. + """ self.add_node(jid, node) self.nodes[(jid, node)]['info'].add_identity( data.get('category', ''), @@ -144,11 +153,27 @@ class StaticDisco(object): data.get('lang', None)) def set_identities(self, jid, node, data): + """ + Add or replace all identities for a JID/node combination. + + The data parameter should include: + identities -- A list of identities in tuple form: + (category, type, name, lang) + """ identities = data.get('identities', set()) self.add_node(jid, node) self.nodes[(jid, node)]['info']['identities'] = identities def del_identity(self, jid, node, data): + """ + Remove an identity from a JID/node combination. + + The data parameter may provide: + category -- The general category to which the agent belonged. + itype -- A more specific designation with the category. + name -- Optional human readable name for this identity. + lang -- Optional, standard xml:lang value. + """ if (jid, node) not in self.nodes: return self.nodes[(jid, node)]['info'].del_identity( @@ -157,21 +182,68 @@ class StaticDisco(object): data.get('name', None), data.get('lang', None)) + def del_identities(self, jid, node, data): + """ + Remove all identities from a JID/node combination. + + The data parameter is not used. + """ + if (jid, node) not in self.nodes: + return + del self.nodes[(jid, node)]['info']['identities'] + def add_feature(self, jid, node, data): + """ + Add a feature to a JID/node combination. + + The data parameter should include: + feature -- The namespace of the supported feature. + """ self.add_node(jid, node) self.nodes[(jid, node)]['info'].add_feature(data.get('feature', '')) def set_features(self, jid, node, data): + """ + Add or replace all features for a JID/node combination. + + The data parameter should include: + features -- The new set of supported features. + """ features = data.get('features', set()) self.add_node(jid, node) self.nodes[(jid, node)]['info']['features'] = features def del_feature(self, jid, node, data): + """ + Remove a feature from a JID/node combination. + + The data parameter should include: + feature -- The namespace of the removed feature. + """ if (jid, node) not in self.nodes: return self.nodes[(jid, node)]['info'].del_feature(data.get('feature', '')) + def del_features(self, jid, node, data): + """ + Remove all features from a JID/node combination. + + The data parameter is not used. + """ + if (jid, node) not in self.nodes: + return + del self.nodes[(jid, node)]['info']['features'] + def add_item(self, jid, node, data): + """ + Add an item to a JID/node combination. + + The data parameter may include: + ijid -- The JID for the item. + inode -- Optional additional information to reference + non-addressable items. + name -- Optional human readable name for the item. + """ self.add_node(jid, node) self.nodes[(jid, node)]['items'].add_item( data.get('ijid', ''), @@ -179,6 +251,13 @@ class StaticDisco(object): name=data.get('name', None)) def del_item(self, jid, node, data): + """ + Remove an item from a JID/node combination. + + The data parameter may include: + ijid -- JID of the item to remove. + inode -- Optional extra identifying information. + """ if (jid, node) in self.nodes: self.nodes[(jid, node)]['items'].del_item( data.get('ijid', ''),