Ticket #2556 defect closed fixed

Opened 7 years ago

Last modified 3 years ago

cfreactor breaks twisted.trial

Reported by: bromine Owned by: bromine
Priority: low Milestone:
Component: core Keywords:
Cc: Branch:
Author: Launchpad Bug:

Description

I have run into a number of problems trying to use cfreactor with twisted.trial.

The biggest problem is that, out of the box, using cfreactor causes twisted.trial to hang. To reproduce this problem, you can put test.py Download and testcase.py Download into a folder and then run

python test.py

from that folder. Python will hang in test.py, which just calls twisted.trial with --reactor cf testcase.py.

This problem is corrected by the attached diff Download. (The issue is that self.didStartRunLoop has to be set before calling runLoop.run() in order for the correct value of self.didStartRunLoop to be available inside the run loop.)

However, even fixing that still leaves two problems with running trial:

twisted/internet/cfreactor.py:294: exceptions.DeprecationWarning: reactor.crash cannot be used inside unit tests. By Twisted 2.7, using crash will fail the test and may crash or hang the test run.

/Software/twisted/trial/reporter.py:219: twisted.trial.reporter.BrokenTestCaseWarning: REACTOR UNCLEAN! traceback(s) follow: 
Traceback (most recent call last):
  File "twisted/trial/unittest.py", line 652, in _classCleanUp
    util._Janitor().postClassCleanup()
  File "twisted/trial/util.py", line 68, in postClassCleanup
    'cleanPending', 'cleanThreads')
  File "twisted/trial/util.py", line 72, in _dispatch
    getattr(self, "do_%s" % attr)()
  File "twisted/trial/util.py", line 126, in do_cleanReactor
    raise DirtyReactorError(' '.join(s))
twisted.trial.util.DirtyReactorError: THIS WILL BECOME AN ERROR SOON! reactor left in unclean state, the following Selectables were left over:  <twisted.internet.posixbase._UnixWaker instance at 0x2a44af8>

Both of these are pointing at a mismatch between the semantics of reactors expected by trial and those actually implemented by cfreactor, but I don't know enough about either to know what the right way to fix this is.

Attachments

cfreactor.py.diff Download (0.5 KB) - added by bromine 7 years ago.
test.py Download (189 bytes) - added by bromine 7 years ago.
testcase.py Download (274 bytes) - added by bromine 7 years ago.

Change History

Changed 7 years ago by bromine

Changed 7 years ago by bromine

Changed 7 years ago by bromine

1

Changed 7 years ago by exarkun

Hmm cfreactor. I wonder if this is a problem that was introduced after it was initially written, or if it has always worked this way.

2

Changed 5 years ago by glyph

  • priority changed from normal to low
  • owner changed from glyph to bromine

cfreactor could really use a maintainer. Any chance you might be willing to take on that role? Starting with a patch for this issue as per TwistedDevelopment, of course? :)

3

Changed 3 years ago by glyph

  • status changed from new to closed
  • resolution set to fixed

(In [30231]) Fix cfreactor. Remove cfsupport.

Author: glyph

Reviewer: exarkun

Fixes: #1833

Fixes: #2556

Fixes: #4652

This change significantly improves cfreactor. It is a rewrite from our broken, unmaintained 'cfsupport' pyrex/C module (which has been removed) to use PyObjC's CoreFoundation and CFNetwork support. This fixes several very serious bugs, including many segfaults.

This also changes the testing status of cfreactor. Previously it received no coverage at all from the buildbots. While this change does not add any unique test code, it adds the CFReactor to all tests which use 'reactorbuilder'. These tests would previously fail (several with segfaults) and they now all pass. The result of this is that most of the direct reactor tests are invoked on our existing OS X buildbot. It's still not 100% supported, since there is still no buildbot running under the cfreactor via 'trial -r cf'. A few tests still fail intermittently in that configuration, but I believe they are all bugs in the tests, rather than in the reactor.

This change also allows Twisted to be built with the 'clang' compiler from the LLVM project. Since 'cfsupport' was the only code that had problems with that compiler, its removal fixes the issue.

(This change also includes several unrelated flymake issues.)

Note: See TracTickets for help on using tickets.