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

dnsdist: Prevent race while creating new TCP worker threads #4764

Merged
merged 2 commits into from Dec 14, 2016

Conversation

@rgacogne
Copy link
Member

@rgacogne rgacogne commented Dec 13, 2016

Short description

We try very hard to avoid using locks, but we need to prevent two threads inserting into the TCP workers vector concurrently. While this can't happen at runtime since the healthcheck thread is the only one calling g_tcpclientthreads->addTCPClientThread(), this might happen at startup time because we start the TCP acceptor threads one after another and they all call it once. This might result, for example, in one vector entry being overwritten and another one remaining value-initialized to zero.

Reported and very kindly investigated by @paddg (thanks!).

Checklist

I have:

We try very hard to avoid using locks, but we need to prevent two
threads inserting into the TCP workers vector concurrently. While
this can't happen at runtime since the healthcheck thread is the
only one calling `g_tcpclientthreads->addTCPClientThread()`, this
might happen at startup time because we start the TCP acceptor
threads one after another and they all call it once.
This might result, for example, in one vector entry being overwritten
and another one remaining value-initialized to zero.
@rgacogne rgacogne added this to the dnsdist-1.1.0 milestone Dec 13, 2016
@rgacogne rgacogne merged commit 3a9f699 into PowerDNS:master Dec 14, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rgacogne rgacogne deleted the rgacogne:dnsdist-tcp-workers-vect-race branch Dec 14, 2016
rgacogne added a commit to rgacogne/pdns that referenced this pull request Dec 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

1 participant