Opened 7 years ago

Closed 7 years ago

#5596 defect closed fixed (fixed)

Twisted Names gethostbyname.py example fails to exit if the host is in /etc/hosts

Reported by: Jean-Paul Calderone Owned by: Jean-Paul Calderone
Priority: normal Milestone:
Component: names Keywords:
Cc: Richard Wall, Thijs Triemstra Branch: branches/gethostbyname-example-5596
branch-diff, diff-cov, branch-cov, buildbot
Author: exarkun

Description

The /etc/hosts resolver operates synchronously, so the gotResult callback in this example runs before reactor.run. The reactor.stop call it contains therefore fails, and there isn't even an errback to handle that failure. The example appears to work, by reporting the address of the host, but then does not exit and does not explain why it is not exiting (ie, reports no error). At least until you hit ^C, and then it displays a traceback and exits.

Should be pretty easily reproducable - just try it on localhost.

This is an example of a very common bug. #1490 and #3270 both aim to solve the bug more generally, so they may be applicable here somehow.

Attachments (5)

gethostbyname-exit-5596.patch (1.5 KB) - added by Richard Wall 7 years ago.
A patch which postpones the lookup until the reactor has started
gethostbyname-exit-5596-2.patch (2.9 KB) - added by Richard Wall 7 years ago.
added better error reporting
gethostbyname-exit-5596-3.patch (2.2 KB) - added by Richard Wall 7 years ago.
Updated to use task.react
gethostbyname-exit-5596-4.patch (9.6 KB) - added by Richard Wall 7 years ago.
Updated the other names examples to use task.react
names-examples-tests-5596.patch (4.0 KB) - added by Richard Wall 7 years ago.
Some basic tests for these examples

Download all attachments as: .zip

Change History (17)

comment:1 Changed 7 years ago by Richard Wall

Cc: Richard Wall added
Owner: set to Richard Wall
Status: newassigned

Yep. The problem is easily recreated.

[richard@zorin trunk]$ python doc/names/examples/gethostbyname.py localhost
127.0.0.1
^CUnhandled error in Deferred:
Unhandled Error
Traceback (most recent call last):
  File "doc/names/examples/gethostbyname.py", line 26, in <module>
    d.addCallbacks(gotResult, gotFailure)
  File "/home/richard/projects/Twisted/trunk/twisted/internet/defer.py", line 290, in addCallbacks
    self._runCallbacks()
--- <exception caught here> ---
  File "/home/richard/projects/Twisted/trunk/twisted/internet/defer.py", line 551, in _runCallbacks
    current.result = callback(current.result, *args, **kw)
  File "doc/names/examples/gethostbyname.py", line 19, in gotResult
    reactor.stop()
  File "/home/richard/projects/Twisted/trunk/twisted/internet/base.py", line 577, in stop
    "Can't stop reactor that isn't running.")
twisted.internet.error.ReactorNotRunning: Can't stop reactor that isn't running.

Changed 7 years ago by Richard Wall

A patch which postpones the lookup until the reactor has started

comment:2 Changed 7 years ago by Richard Wall

Keywords: review added
Owner: Richard Wall deleted
Status: assignednew

Ready for review in attachment:gethostbyname-exit-5596.patch

  • Call getHostByName via reactor.callWhenRunning so that reactor is started and stopped in the correct order.
  • Added some doc comments explaining how the default resolver works and explaining that hosts lookup is synchronous.

Output now looks like this:

[richard@zorin gethostbyname-exit-5596]$ python doc/names/examples/gethostbyname.py localhost
127.0.0.1

[richard@zorin gethostbyname-exit-5596]$ python doc/names/examples/gethostbyname.py www.example.com
192.0.43.10

[richard@zorin gethostbyname-exit-5596]$ python doc/names/examples/gethostbyname.py does.not.exist
Traceback (most recent call last):
Failure: twisted.names.error.DNSNameError: <twisted.names.dns.Message instance at 0x2a422d8>

comment:3 in reply to:  2 ; Changed 7 years ago by Thijs Triemstra

Cc: Thijs Triemstra added
Keywords: review removed
Owner: set to Richard Wall

Replying to rwall:

[richard@zorin gethostbyname-exit-5596]$ python doc/names/examples/gethostbyname.py does.not.exist Traceback (most recent call last): Failure: twisted.names.error.DNSNameError: <twisted.names.dns.Message instance at 0x2a422d8>

Thanks for the patch. Is it possible to print more details for that Failure: twisted.names.error.DNSNameError: <twisted.names.dns.Message instance at 0x2a422d8>? I assume someone will want to see more details when encountering a failure..

Changed 7 years ago by Richard Wall

added better error reporting

comment:4 in reply to:  3 Changed 7 years ago by Richard Wall

Keywords: review added

Replying to thijs:

Thanks for the patch. Is it possible to print more details for that Failure: twisted.names.error.DNSNameError: <twisted.names.dns.Message instance at 0x2a422d8>? I assume someone will want to see more details when encountering a failure.

Thanks for the review. I agree and have added better error messages.

Have a look at attachment:gethostbyname-exit-5596-2.patch

Output now looks like this:

$ python gethostbyname.py www.google.com
2a00:1450:400c:c05::67
$ python gethostbyname.py www.google.com
173.194.67.104
$ python gethostbyname.py localhost
127.0.0.1
$ python gethostbyname.py com
ERROR: No IP adresses found for hostname 'com'
$ python gethostbyname.py does.not.exist
ERROR: hostname not found 'does.not.exist'

Changed 7 years ago by Richard Wall

Updated to use task.react

comment:5 Changed 7 years ago by Richard Wall

See attachment:gethostbyname-exit-5596-3.patch

  • Updated again to use exarkun's new task.react function from #3270

Changed 7 years ago by Richard Wall

Updated the other names examples to use task.react

comment:6 Changed 7 years ago by Richard Wall

Another patch attachment:gethostbyname-exit-5596-4.patch

  • Use task.react in all the names example scripts
  • use consistent names and documentation for all the names examples
  • Tidied up the output and error reporting of dns-service.py

Also fixes: #5989

comment:7 Changed 7 years ago by Richard Wall

Owner: Richard Wall deleted

Changed 7 years ago by Richard Wall

Some basic tests for these examples

comment:8 Changed 7 years ago by Richard Wall

In attachment:names-examples-tests-5596.patch

  • Added tests for consistent USAGE documentation and command line error reporting
  • Added tests for standard python shebang line and that the script is executable.

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

Author: exarkun
Branch: branches/gethostbyname-example-5596

(In [36679]) Branching to 'gethostbyname-example-5596'

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

(In [36680]) Apply part of gethostbyname-exit-5596-4.patch

refs #5596

Drop all the parts not related to the ticket description

comment:11 Changed 7 years ago by Jean-Paul Calderone

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

Thanks. To simplify the review, I only considered the part of the patch which fixes the problem described by the ticket summary.

That parts looks great. I'll apply it to trunk. The other parts should be attached to new (or existing) tickets.

comment:12 Changed 7 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

(In [36682]) Merge gethostbyname-example-5596

Author: rwall Reviewer: exarkun Fixes: #5596

Reliably exit the gethostbyname.py Twisted Names example by using twisted.internet.task.react to run and stop the reactor, rather than doing it directly in an error prone manner.

Note: See TracTickets for help on using tickets.