Opened 6 years ago

Closed 6 years ago

Last modified 6 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
(diff, github, buildbot, log)
Author: ralphm Launchpad Bug:

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 6 years ago by jgill

  • Component changed from core to words
  • Owner changed from glyph to exarkun

Part of words package.

comment:2 Changed 6 years ago by ralphm

  • Owner changed from exarkun 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 6 years ago by ralphm

  • author set to ralphm
  • Branch set to branches/xish-nested-dispatch-3066

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

comment:4 Changed 6 years ago by ralphm

  • Cc jgill ralphm added
  • Keywords review added
  • Milestone set to Words 8.0.0 + 1
  • Owner ralphm deleted
  • Priority changed from normal to highest

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 6 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 6 years ago by ralphm

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

:-( Sorry about that. Fixed now.

comment:7 Changed 6 years ago by therve

  • Keywords review removed
  • Owner changed from therve to ralphm
  • Priority changed from highest to high

Great, please merge.

comment:8 Changed 6 years ago by ralphm

  • Resolution set to fixed
  • Status changed from new to closed

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

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

comment:9 Changed 6 years ago by trac

  • Milestone Words 8.0.0 + 1 deleted

Milestone Words 8.0.0 + 1 deleted

comment:10 Changed 4 years ago by <automation>

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