Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#3066 defect closed fixed (fixed)

twisted.words.xish.utility.addOnetimeObserver can register a permanent xml stream observer

Reported by: jgill Owned by:
Priority: high Milestone:
Component: words Keywords:
Cc: jgill, ralphm, therve Branch: branches/xish-nested-dispatch-3066
branch-diff, diff-cov, branch-cov, buildbot
Author: ralphm

Description

twisted.words.xish.utility.addOnetimeObserver() adds an observer to an XPath event on an xml stream. Under some situations the method will defer appending the callback to the callback queue, e.g. if is in the middle of a dispatch call. The queued command, however, calls addObserver() rather than _addObserver(), and therefore drops the 'onetime' flag.

A simple fix might be to change the lambda call near the start of _addObserver() from

lambda:self.addObserver(event, observerfn, priority, *args, kwargs)

to:

lambda:self._addObserver(onetime, event, observerfn, priority, *args, kwargs)

Change History (10)

comment:1 Changed 10 years ago by jgill

Component: corewords
Owner: changed from Glyph to Jean-Paul Calderone

Part of words package.

comment:2 Changed 10 years ago by ralphm

Owner: changed from Jean-Paul Calderone to ralphm

Reading that piece of code, this indeed seems to be the case. Do you have a test case that shows this behavior?

comment:3 Changed 10 years ago by ralphm

author: ralphm
Branch: branches/xish-nested-dispatch-3066

(In [23387]) Branching to 'xish-nested-dispatch-3066'

comment:4 Changed 10 years ago by ralphm

Cc: jgill ralphm added
Keywords: review added
Milestone: Words 8.0.0 + 1
Owner: ralphm deleted
Priority: normalhighest

I made up the test case myself and fixed the one for the non-onetime observer to actually test that.

Please review.

comment:5 Changed 10 years ago by therve

Keywords: review removed
Owner: set to ralphm

2 tests are failing:

[ERROR]: twisted.words.test.test_xishutil.EventDispatcherTest.testStuff

Traceback (most recent call last):
  File "/home/therve/Projets/Twisted/trunk/twisted/words/test/test_xishutil.py", line 80, in testStuff
    self.assertEquals(cb1.object, msg)
exceptions.AttributeError: CallbackTracker instance has no attribute 'object'
===============================================================================
[ERROR]: twisted.words.test.test_xishutil.EventDispatcherTest.test_addObserverTwice

Traceback (most recent call last):
  File "/home/therve/Projets/Twisted/trunk/twisted/words/test/test_xishutil.py", line 113, in test_addObserverTwice
    self.assertEquals(cb1.object, d)
exceptions.AttributeError: CallbackTracker instance has no attribute 'object'

comment:6 Changed 10 years ago by ralphm

Cc: therve added
Keywords: review added
Owner: changed from ralphm to therve

:-( Sorry about that. Fixed now.

comment:7 Changed 10 years ago by therve

Keywords: review removed
Owner: changed from therve to ralphm
Priority: highesthigh

Great, please merge.

comment:8 Changed 10 years ago by ralphm

Resolution: fixed
Status: newclosed

(In [23418]) Fix adding one-time observers while in dispatch.

Author: jgill, ralphm Reviewer: therve Fixes #3066.

comment:9 Changed 9 years ago by trac

Milestone: Words 8.0.0 + 1

Milestone Words 8.0.0 + 1 deleted

comment:10 Changed 7 years ago by <automation>

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