Opened 8 years ago

Closed 8 years ago

#3718 defect closed fixed (fixed)

twisted.internet.posixbase._Waker not defined on Jython

Reported by: pjenvey Owned by:
Priority: normal Milestone:
Component: core Keywords:
Cc: Thijs Triemstra, therve Branch: branches/jython-waker-3718
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description (last modified by Thijs Triemstra)

posixbase defines a _Waker depending on the value of twisted.python.runtime.platformType, which is 'java' on Jython. There's no _Waker definition for 'java'

$ jython -W all ./bin/trial --reporter=bwverbose --unclean-warnings twisted
<snip>
Traceback (most recent call last):
  File "./bin/trial", line 24, in <module>
    run()
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/scripts/trial.py", line 361, in run
    test_result = trialRunner.run(suite)
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/trial/runner.py", line 813, in run
    return self._runWithoutDecoration(test)
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/trial/runner.py", line 839, in _runWithoutDecoration
    run()
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/trial/runner.py", line 834, in <lambda>
    run = lambda: suite.run(result)
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/trial/runner.py", line 293, in run
    self._bail()
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/trial/runner.py", line 275, in _bail
    from twisted.internet import reactor
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/internet/reactor.py", line 38, in <module>
    selectreactor.install()
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/internet/selectreactor.py", line 200, in install
    reactor = SelectReactor()
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/internet/selectreactor.py", line 72, in __init__
    posixbase.PosixReactorBase.__init__(self)
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/internet/posixbase.py", line 168, in __init__
    ReactorBase.__init__(self)
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/internet/base.py", line 467, in __init__
    self._initThreads()
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/internet/base.py", line 887, in _initThreads
    self.installWaker()
  File "/Users/pjenvey/src/java/jython/jython-twisted/src/Twisted/twisted/internet/posixbase.py", line 204, in installWaker
    self.waker = _Waker(self)
NameError: global name '_Waker' is not defined

Attachments (1)

waker.patch (1.1 KB) - added by sanxiyn 8 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 8 years ago by Thijs Triemstra

Cc: Thijs Triemstra added

comment:2 Changed 8 years ago by Thijs Triemstra

Description: modified (diff)
Summary: twisted.internet.posixbase._Walker not defined on Jythontwisted.internet.posixbase._Waker not defined on Jython

Updating ticket description, fixing typo (Waker instead of Walker)

comment:3 Changed 8 years ago by Thijs Triemstra

The buildslave is reporting the same error message now.

Changed 8 years ago by sanxiyn

Attachment: waker.patch added

comment:4 Changed 8 years ago by sanxiyn

Keywords: review added

Please review.

comment:5 Changed 8 years ago by Jean-Paul Calderone

Owner: Glyph deleted

comment:6 Changed 8 years ago by sanxiyn

Oops, os.pipe is present on win32. Is "I am a workaround for the lack of pipes on win32" accurate?

comment:7 Changed 8 years ago by therve

Cc: therve added
Keywords: review removed
Owner: set to sanxiyn

Yeah, pipes are available on windows, but are not selectable (iirc). So you need to check for something else, unfortunately. And indeed the docstring of Win32Waker should be updated.

comment:8 Changed 8 years ago by Jean-Paul Calderone

Author: exarkun
Branch: branches/jython-waker-3718

(In [28218]) Branching to 'jython-waker-3718'

comment:9 Changed 8 years ago by Jean-Paul Calderone

(In [28219]) Use the newly renamed _SocketWaker on Windows, _PipeWaker everywhere else

refs #3718

comment:10 Changed 8 years ago by Jean-Paul Calderone

Keywords: review added
Owner: sanxiyn deleted

Fixed some of that stuff. Jython can run the tests in the branch.

comment:11 Changed 8 years ago by Thijs Triemstra

I{self-pipe trick<http://cr.yp.to/docs/selfpipe.html>} should probably use U instead of I.

comment:12 in reply to:  10 Changed 8 years ago by Thijs Triemstra

Replying to exarkun:

Fixed some of that stuff. Jython can run the tests in the branch.

Trial is dying with:

Exception in thread "MainThread" java.lang.OutOfMemoryError: Java heap space

Exarkun, could you increase the memory? From #jython:

pjenvey: default is 512mb, you can do jython -J-Xmx1024m
pjenvey: JAVA_MEM=-Xmx1024m jython should work too

comment:13 Changed 8 years ago by Thijs Triemstra

Keywords: review removed
Owner: set to Jean-Paul Calderone

I see you added a 1024, but looks like it's still not enough. Can you double it?

comment:14 Changed 8 years ago by Thijs Triemstra

I just ran it (on that buildslave) with 2048 and it still fails. I guess the buildslave, or jython support, is not an option until we take a look at trial's memory usage.

comment:15 Changed 8 years ago by Jean-Paul Calderone

One possible explanation for the memory usage is that all the failures trial is keeping around are responsible. This might be helped by throwing away the actual failures and just keeping the string that will be reported at the end.

If this is the cause, then another possible work-around would be to run a limited subset of the tests on Jython until those all pass, then expand that set and repeat.

comment:16 Changed 8 years ago by Jean-Paul Calderone

Keywords: review added
Owner: Jean-Paul Calderone deleted

I set up the builder to just run twisted.test for now. Let's see what happens...

comment:17 Changed 8 years ago by therve

Keywords: review removed
Owner: set to Jean-Paul Calderone

Looks better, I guess. +1.

comment:18 Changed 8 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [28292]) Merge jython-waker-3718

Author: sanxiyn, exarkun Reviewer: therve Fixes: #3718

Change the waker selection code to be aware of non-Windows, non-POSIX platforms (such as Java/Jython).

comment:19 Changed 7 years ago by <automation>

Owner: Jean-Paul Calderone deleted
Note: See TracTickets for help on using tickets.