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.

Sign up for GitHub

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

Jump to bottom

CommunicatorClass::slaveRefresh should verify if Zone is still configured in backend #596

Closed
Habbie opened this issue Apr 26, 2013 · 5 comments
Closed

CommunicatorClass::slaveRefresh should verify if Zone is still configured in backend #596

Habbie opened this issue Apr 26, 2013 · 5 comments
Assignees
@Habbie
Labels
auth defect

Comments

@Habbie
Copy link
Member

Habbie commented Apr 26, 2013

/var/log/messages:Oct 8 11:05:27 ns1 pdns[5740]: Initiating transfer of 'foo.bar' from remote '1.2.3.4'

/var/log/messages:Oct 8 11:05:27 ns1 pdns[5740]: Can't determine backend for domain 'foo.bar'

We suspect a race condition where CommunicatorClass::slaveRefresh
does not check if the (database)backend if the domain is still there or
has already been deleted.
The return value of B->getDomainInfo(di.zone, di); has to be verified.
@ghost ghost assigned Habbie Apr 26, 2013
@Habbie Habbie closed this as completed Apr 26, 2013
Habbie pushed a commit that referenced this issue Apr 26, 2013
avoid race condition, also avoid a rare crash. Patch from ticket #596
f9fa0e2 
git-svn-id: svn://svn.powerdns.com/pdns/trunk/pdns@2802 d19b8d6e-7fed-0310-83ef-9ca221ded41b
@Habbie
Copy link
Member Author

Habbie commented Apr 26, 2013

Attachment 'Fix' (slavecommunicator.patch) https://gist.github.com/5466801

@Habbie
Copy link
Member Author

Habbie commented Apr 26, 2013

Author: peter
This patch looks like it just shortens the race a bit, at the cost of additional database queries for every transfer. I'm not sure this makes economic sense. Please comment :)

@Habbie
Copy link
Member Author

Habbie commented Apr 26, 2013

Author: anon
Replying to [comment:1 peter]:

This patch looks like it just shortens the race a bit, at the cost of additional database
queries for every transfer. I'm not sure this makes economic sense. Please comment :)

B->getDomainInfo(di.zone, di) gets called in both versions anyway, the only thing we do is verifying it's return code. In the race we even reduce the overhead when getDomainInfo returns false.

@Habbie
Copy link
Member Author

Habbie commented Apr 26, 2013

Author: peter
you're right, I misread that. Looks good!

@Habbie
Copy link
Member Author

Habbie commented Apr 26, 2013

Author: peter
applied in r2802, thanks!

mind04 pushed a commit to mind04/pdns that referenced this issue Apr 26, 2013
avoid race condition, also avoid a rare crash. Patch from ticket Powe…
2a52895 
…rDNS#596

git-svn-id: svn://svn.powerdns.com/pdns/trunk/pdns@2802 d19b8d6e-7fed-0310-83ef-9ca221ded41b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Habbie Habbie

Labels
auth defect
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@Habbie