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

dns: occasionally crashing in resolve4 and setServers #14734

Closed
XadillaX opened this issue Aug 10, 2017 · 2 comments
Closed

dns: occasionally crashing in resolve4 and setServers #14734

XadillaX opened this issue Aug 10, 2017 · 2 comments
Assignees
Labels
cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. confirmed-bug Issues with confirmed bugs. dns Issues and PRs related to the dns subsystem.

Comments

@XadillaX
Copy link
Contributor

  • Version: v9.0.0-pre (master)
  • Platform: Darwin zanarpro 16.6.0 Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64 i386 MacBookPro11,3 Darwin
  • Subsystem: c-ares

Here's the code:

// under test folder
const common = require('../common');
const dns = require('dns');

dns.resolve4('google.com', common.mustCall(function(/* err, nameServers */) {
  dns.setServers([ '8.8.8.8' ]);
}));

dns.resolve4('google.com', common.mustCall(function() {
  // do nothing...
}));

And the Node.js will occasionally crash with:

Assertion failed: (ares__is_list_empty(&server->queries_to_server)), function ares__destroy_servers_state, file ../deps/cares/src/ares_destroy.c, line 102.
@XadillaX
Copy link
Contributor Author

XadillaX commented Aug 10, 2017

I'm going to fix this issue. I think I know where the problem is, but I need some time to fix it.

@XadillaX XadillaX self-assigned this Aug 10, 2017
@XadillaX XadillaX added cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. confirmed-bug Issues with confirmed bugs. dns Issues and PRs related to the dns subsystem. labels Aug 10, 2017
XadillaX added a commit to XadillaX/node that referenced this issue Nov 15, 2017
Fix this issue follow these two points:

1. Keep track of how many queries are currently open. If `setServers()`
   is called while there are open queries, error out.
2. For `Resolver` instances, use option 1. For dns.setServers(), just
   create a fresh new default channel every time it is called, and then
   set its servers list.

Fixes: nodejs#14734
MylesBorins pushed a commit that referenced this issue Dec 12, 2017
Fix this issue follow these two points:

1. Keep track of how many queries are currently open. If `setServers()`
   is called while there are open queries, error out.
2. For `Resolver` instances, use option 1. For dns.setServers(), just
   create a fresh new default channel every time it is called, and then
   set its servers list.

PR-URL: #14891
Fixes: #14734
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this issue Dec 12, 2017
Fix this issue follow these two points:

1. Keep track of how many queries are currently open. If `setServers()`
   is called while there are open queries, error out.
2. For `Resolver` instances, use option 1. For dns.setServers(), just
   create a fresh new default channel every time it is called, and then
   set its servers list.

PR-URL: #14891
Fixes: #14734
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
XadillaX added a commit to XadillaX/node that referenced this issue Jan 25, 2018
Fix this issue follow these two points:

1. Keep track of how many queries are currently open. If `setServers()`
   is called while there are open queries, error out.
2. For `Resolver` instances, use option 1. For dns.setServers(), just
   create a fresh new default channel every time it is called, and then
   set its servers list.

Fixes: nodejs#14734
gibfahn pushed a commit that referenced this issue Feb 19, 2018
Fix this issue follow these two points:

1. Keep track of how many queries are currently open. If `setServers()`
   is called while there are open queries, error out.
2. For `Resolver` instances, use option 1. For dns.setServers(), just
   create a fresh new default channel every time it is called, and then
   set its servers list.

Fixes: #14734
PR-URL: #14891
Backport-PR-URL: #17778
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@dzek69
Copy link

dzek69 commented Jun 16, 2018

I monkey patched dns module to fix that. If anyone needs to keep unpatched Node version and doesn't want the crashes - please use my fix.
More information here: https://github.com/dzek69/node-dns-bugfix

BTW: It's a duplicate of #894.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cares Issues and PRs related to the c-ares dependency or the cares_wrap binding. confirmed-bug Issues with confirmed bugs. dns Issues and PRs related to the dns subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants