test failures with message "exceptions.AttributeError: 'StreamServerEndpointService' object has no attribute '_port'", and failure to write node.url file with Twisted 10.2

[davidsarah]

​http://tahoe-lafs.org/buildbot/builders/Zooko%20zomp%20Mac-amd64%2010.6%20py2.6/builds/264/steps/test/logs/stdio

I'll refrain from speculation in the ticket Description, except to say that this may be specific to Twisted 10.2.

[davidsarah]

OK, here's the speculation.

The exception is thrown from src/allmydata/test/no_network.py@4657#L290:

self.client_webports = [c.getServiceNamed("webish").listener._port.getHost().port
                        for c in self.g.clients]

where the .listener variable was set at src/allmydata/webish.py@4672#L152:

s = strports.service(webport, site)
s.setServiceParent(self)
self.listener = s # stash it so the tests can query for the portnum

On Twisted trunk-at-time-of-writing (latest release 10.2), strports.service is defined ​here. Notice that it returns an instance of ​StreamServerEndpointService. Prior to ​Twisted changeset 30155, it returned an instance of some other dynamically-chosen Server class.

Neither StreamServerEndpointService nor its superclass ​Service have a _port instance variable. From instrumenting webish.py and running with Twisted 10.0, the listener was previously an instance of twisted.application.internet.TCPServer, which inherits from ​_AbstractServer, which does have a _port variable.

I think this should only affect tests, since Tahoe does not reference _port in any non-test code.

[davidsarah]

Replying to davidsarah:

I think this should only affect tests, since Tahoe does not reference _port in any non-test code.

I'm mistaken; webish.py references _port in its _write_nodeurl_file callback method. That method also depends incorrectly on the listener being an instance of twisted.application.internet.{TCPServer,SSLServer}.

[davidsarah]

Brian suggested that this was the same bug as ​http://foolscap.lothar.com/trac/ticket/167 . It's certainly caused by the same change to Twisted, but I think that Tahoe needs an independent fix (as well as being updated to require foolscap 0.6.0). Note that the problem in this ticket occurs when starting a web-API server, not a storage server.

[davidsarah]

Ugly, ugly fix for #1286. There must be a better way than relying on internal _-prefixed variables in Twisted. Lacks tests, do not apply yet.

[davidsarah]

c82b48f3d60fdfbc updates the foolscap version requirement to 0.6.0. zomp is off-line right now, but I don't expect that update to fix this ticket.

[warner]

oops, yes, I think this is a separate instance of the problem in ​foolscap#167. I think we can get away with using the same solution: roll our own strports() parser, but keep using TCPServer (and keep relying upon the internal ._port attribute). The fix is harder than for Foolscap, however, because we were using the strports parser to make our webapi SSL capable: the new parser we write needs to also handle the keyfile= or whatever magic argument turned on SSL.

[zooko]

zomp is back, and indeed it still fails:

​http://tahoe-lafs.org/buildbot/builders/Zooko%20zomp%20Mac-amd64%2010.6%20py2.6/builds/271/steps/test/logs/stdio

allmydata.test.test_checker.AddLease.test_875 ... Traceback (most recent call last):
  File "/Users/tahoebuildslave/Zooko zomp Mac-amd64 10.6 py2.6/build/src/allmydata/test/test_checker.py", line 281, in test_875
    self.set_up_grid(num_servers=1)
  File "/Users/tahoebuildslave/Zooko zomp Mac-amd64 10.6 py2.6/build/src/allmydata/test/no_network.py", line 291, in set_up_grid
    for c in self.g.clients]
exceptions.AttributeError: 'StreamServerEndpointService' object has no attribute '_port'
[ERROR]
(0.134 secs)

[davidsarah]

I think the original patch isn't correct because it is only getting the port as parsed from the strport specification, which may be zero, not the actual port on which we are listening. (The existing tests seem not to be sufficiently complete to catch that.)

[warner]

functioning patch: tolerate 10.2 and earlier versions

[davidsarah]

The _ and _ or _ syntax is Python 2.5+. Otherwise LGTM.

(warner convinced me that there was no possibility of plan interference from calling .callback and .errback synchronously.)

[Brian Warner <warner@…>]

In 09a2241471c56f0a:

(The changeset message doesn't reference this ticket)

[davidsarah]

Reading over this patch again, some cleanups are possible:

The Deferred in self._started to which _write_nodeurl_file is added as a callback, is fired just after setting self._url. So the "if self._url:" test in _write_nodeurl_file is not needed. (If we use getURL(), the code will assert if there is any bug that makes me wrong about this, causing an error to be logged.)

We can use fileutil.write instead of open+write. Also it's clearer to make the _write_nodeurl_file a local function rather than a method.

The comment "# this file is world-readable" is wrong in general; it will have the default permissions for files created by the node process (or the permissions are not changed if the file already exists).

Finally, .listener is intended to be private so it should probably be ._listener.

[davidsarah]

rc/allmydata/webish.py: clean-ups and correction to a comment. Also change an open and write to use fileutil.write. See ref #1286 comment 13.

[zooko]

attachment:webish-cleanup.darcs.patch​ looks good to me!

[Brian Warner <warner@…>]

In [4979/ticket1306]:

Tolerate Twisted-10.2's endpoints, patch by David-Sarah. Closes #1286.

The service generated by strports.service() changed in 10.2, and the ugly
private-attribute-reading hack we used to glean a kernel-allocated port
number (e.g. when using "tcp:0", especially during unit tests) broke, causing
Tahoe to be completely unusable with Twisted-10.2 . The new ugly
private-attribute-reading hack starts by figuring out what sort of service
was generated, then reads different attributes accordingly.

This also hushes a warning when using schemeless strports strings like "0" or
"3456", by quietly prepending a "tcp:" scheme, since 10.2 complains about
those. It also adds getURL() and getPortnum() accessors to the "webish"
service, rather than having unit tests dig through _url and _portnum and such
to find out what they are.