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

New Issue

tahoe-lafs · 2010-12-31T01:24:04Z

davidsarah commented

2010-12-31 01:24:04 +00:00

(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.

(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.

tahoe-lafs added the

labels 2010-12-31 01:24:04 +00:00

tahoe-lafs added this to the 1.9.0 milestone 2010-12-31 01:24:04 +00:00

davidsarah commented

2010-12-31 02:15:02 +00:00

OK, here's the speculation.

The exception is thrown from source: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 source: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.

OK, here's the speculation. The exception is thrown from source: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 source: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](http://twistedmatrix.com/trac/browser/trunk/twisted/application/strports.py?rev=30265#L38). Notice that it returns an instance of [StreamServerEndpointService](http://twistedmatrix.com/trac/browser/trunk/twisted/application/internet.py?rev=30265#L321). Prior to [Twisted changeset 30155](http://twistedmatrix.com/trac/changeset/30155#file1), it returned an instance of some other dynamically-chosen Server class. Neither `StreamServerEndpointService` nor its superclass [Service](http://twistedmatrix.com/trac/browser/trunk/twisted/application/service.py#L153) 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](http://twistedmatrix.com/trac/browser/trunk/twisted/application/internet.py#L72), 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 commented

2010-12-31 03:33:31 +00:00

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]source:src/allmydata/webish.py@4676#L163 callback method. That method also depends incorrectly on the listener being an instance of twisted.application.internet.{TCPServer,SSLServer}.

Replying to [davidsarah](/tahoe-lafs/trac-2024-07-25/issues/1286#issuecomment-81631): > 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]source:src/allmydata/webish.py@4676#L163 callback method. That method also depends incorrectly on the `listener` being an instance of `twisted.application.internet.{TCPServer,SSLServer`}.

tahoe-lafs changed title from ~~test failures with message "exceptions.AttributeError: 'StreamServerEndpointService' object has no attribute '_port'"~~ to test failures with message "exceptions.AttributeError: 'StreamServerEndpointService' object has no attribute '_port'", and failure to write node.url file with Twisted 10.2

2010-12-31 03:36:15 +00:00

davidsarah commented

2010-12-31 04:46:45 +00:00

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.

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 commented

2010-12-31 06:33:06 +00:00

Attachment ugly-fix-for-1286.darcs.patch (8591 bytes) added

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.

**Attachment** ugly-fix-for-1286.darcs.patch (8591 bytes) added 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.

ugly-fix-for-1286.darcs.patch

8.4 KiB

davidsarah commented

2010-12-31 06:42:10 +00:00

changeset: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 commented

2011-01-05 00:26:55 +00:00

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.

oops, yes, I think this is a separate instance of the problem in [foolscap#167](http://foolscap.lothar.com/trac/ticket/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.

tahoe-lafs modified the milestone from 1.9.0 to 1.8.2

2011-01-06 00:33:58 +00:00

zooko commented

2011-01-08 17:28:51 +00:00

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)

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 commented

2011-01-15 00:14:31 +00:00

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 commented

2011-01-17 07:06:29 +00:00

Attachment 1286.diff (6719 bytes) added

functioning patch: tolerate 10.2 and earlier versions

**Attachment** 1286.diff (6719 bytes) added functioning patch: tolerate 10.2 and earlier versions

1286.diff

6.6 KiB

davidsarah commented

2011-01-17 07:19:46 +00:00

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.)

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@lothar.com> commented

2011-01-17 08:20:57 +00:00

In changeset:09a2241471c56f0a:

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.

In changeset:09a2241471c56f0a: ``` 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. ```

tahoe-lafs added the

fixed

label 2011-01-17 08:20:57 +00:00

tahoe-lafs closed this issue

2011-01-17 08:20:57 +00:00

davidsarah commented

2011-01-18 00:06:14 +00:00

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.

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 commented

2011-01-18 00:07:36 +00:00

Attachment webish-cleanup.darcs.patch (18190 bytes) added

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.

**Attachment** webish-cleanup.darcs.patch (18190 bytes) added 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.

webish-cleanup.darcs.patch

18 KiB

zooko commented

2011-01-18 07:51:24 +00:00

webish-cleanup.darcs.patch looks good to me!

[webish-cleanup.darcs.patch](/tahoe-lafs/trac-2024-07-25/attachments/000078ac-91b9-2669-ecf0-5ca046e33285) looks good to me!

Brian Warner <warner@lothar.com> commented

2011-01-20 10:05:00 +00:00

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.

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. ```

Sign in to join this conversation.