Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"not reached" panic in server.go #25

Closed
abh opened this issue Sep 8, 2012 · 6 comments
Closed

"not reached" panic in server.go #25

abh opened this issue Sep 8, 2012 · 6 comments

Comments

@abh
Copy link
Contributor

abh commented Sep 8, 2012

Since upgrading to the latest version some of my boxes are crashing with this error. Looking at the code it seems impossible, so not sure what's going on. I wish there was an easy way to know exactly which version of the dns package is included or otherwise a better way to manage the package dependencies. :-/

geodns 00:33:23.263167 serve.go:139: Opening on 64.142.112.226:53 udp
geodns 00:33:23.263202 serve.go:139: Opening on 207.171.17.42:53 udp
geodns 00:33:23.263187 serve.go:139: Opening on 64.142.112.226:53 tcp
geodns 00:33:23.263215 serve.go:139: Opening on 207.171.17.42:53 tcp
geodns 00:33:23.268425 config.go:47: Updated file, going to read pool.ntp.org.json
panic: dns: not reached

goroutine 17 [running]:
github.com/miekg/dns.(*ServeMux).match(0x188571c8, 0x18bb6340, 0xc, 0x6f0001, 0x29, ...)
    /home/ask/go/src/github.com/miekg/dns/server.go:196 +0x10c
github.com/miekg/dns.(*ServeMux).ServeDNS(0x188571c8, 0x18bb7690, 0x18bb9000, 0x18bba000, 0x18bb9000, ...)
    /home/ask/go/src/github.com/miekg/dns/server.go:230 +0xe8
github.com/miekg/dns.(*conn).serve(0x18bb7000, 0x0)
    /home/ask/go/src/github.com/miekg/dns/server.go:419 +0x28e
created by github.com/miekg/dns.(*Server).serveUDP
    /home/ask/go/src/github.com/miekg/dns/server.go:377 +0x310

goroutine 1 [chan receive]:
...
@abh
Copy link
Contributor Author

abh commented Sep 8, 2012

for what it's worth I got it to work again by rolling dns back to 9b74e1b (and radix to the version from September 4).

@miekg
Copy link
Owner

miekg commented Sep 8, 2012

[ Quoting notifications@github.com in "Re: [dns] "not reached" panic in se..." ]

for what it's worth I got it to work again by rolling dns back to 9b74e1b (and
radix to the version from September 4).

Yeah. I will introduce some versioning in both radix and godns. So that
there is a v2 of both in which I do my thing and the current stuff, which
is then production.

Thanks for the report though.

Regards,

Miek Gieben                                                   http://miek.nl

@miekg
Copy link
Owner

miekg commented Sep 8, 2012

[ Quoting notifications@github.com in "[dns] "not reached" panic in server..." ]

Since upgrading to the latest version some of my boxes are crashing with this
error. Looking at the code it seems impossible, so not sure what's going on. I

No, the panic can still be hit. It must be a 'return nil'. It is triggered if
you receive a question for a zone you have no knowledge of.

wish there was an easy way to know exactly which version of the dns package is
included or otherwise a better way to manage the package dependencies. :-/

The biggest stuff (axfr, signing, radix updates: Next() and Prev()) is now
working. So I'm only fiddling with the new stuff as of now.

Best way forward is to check/run the current code, stabelize that a bit more (if
needed). In the mean time I'll add a v2 dir (just as the mgo driver did).

Regards,

Miek Gieben                                                   http://miek.nl

@abh
Copy link
Contributor Author

abh commented Sep 8, 2012

I should have made a separate ticket or email about the versioning thing. I don't want to discourage you iterating on the library. There are so few users now, so I'd rather just you make it better-better-better.

For the panic: So it's crashing because it's listening and then getting a query for a "unknown" domain because the zones aren't loaded yet? (I do the listening and the loading in separate goroutines). That sounds crazy and glancing at the git diff's I don't see how/when that changed. The desired behavior (IMO) is to just return SERVFAIL if the server doesn't know what to do with the request.

I'd rather have my servers return SERVFAIL quickly so the client can try another than having it timeout.

@abh abh mentioned this issue Sep 8, 2012
@miekg
Copy link
Owner

miekg commented Sep 9, 2012

[ Quoting notifications@github.com in "Re: [dns] "not reached" panic in se..." ]

For the panic: So it's crashing because it's listening and then getting a query
for a "unknown" domain because the zones aren't loaded yet? (I do the listening
and the loading in separate goroutines).

The panic is now gone, so it will return servfail if the zone's name isn't
registred in the servers handler. I.e. if now handlers are found the
servfail handler is invoked (automatically).

That sounds crazy and glancing at the
git diff's I don't see how/when that changed. The desired behavior (IMO) is to
just return SERVFAIL if the server doesn't know what to do with the request.

I'd rather have my servers return SERVFAIL quickly so the client can try
another than having it timeout.

it does what you think it should do :-) To ellaborate a bit:

  1. why is the server's zonelist a radix tree in the first place? Well, it
    was a map, but this makes searching for parent zones (as you must for
    DS records) slow. And I think a radix tree will be faster if you have
    a lot of zones (millions).
  2. If the code finds an exact hit where done, if the hit wasn't exact
    we have found a potential parent node/zone. This must be checked to
    see if you need to give out an referral or not.
    But the radix tree search might have returned an intermediate node with no
    values attached to it. This is why we go Up() the tree. This terminates at
    the root of the tree, which is also a node with no value. At this point
    that if-then-else is finished and we drop down to the 'return nil'

grtz Miek

Regards,

Miek Gieben                                                   http://miek.nl

@abh
Copy link
Contributor Author

abh commented Sep 9, 2012

Thank you for fixing it! I saw the fix this morning and I am building with the latest dns package again now.

@abh abh closed this as completed Sep 9, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants