fix register type race

[omoerbeek]

Short description

Actually seen in recursor, but auth could also have it in theory.

The issue is that you cannot call type register functions while the rest of the threads are doing their work. The actual fix is in reczones.cc, but also guard against this happening in other places (for both rec and auth), and while there remove the unused unregist function.

Checklist

I have:

• read the CONTRIBUTING.md document
• compiled this code
• tested this code
• included documentation (including possible behaviour changes)
• documented the code
• added or modified regression test(s)
• added or modified unit test(s)

[coveralls]

Pull Request Test Coverage Report for Build 12082288051

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 77 of 77 (100.0%) changed or added relevant lines in 6 files are covered.
• 60 unchanged lines in 12 files lost coverage.
• Overall coverage decreased (-8.4%) to 64.699%

---

Files with Coverage Reduction	New Missed Lines	%
pdns/packethandler.cc	1	72.35%
pdns/misc.cc	1	63.88%
pdns/recursordist/sortlist.cc	2	74.12%
pdns/recursordist/test-syncres_cc2.cc	3	88.91%
pdns/iputils.hh	3	78.34%
pdns/rcpgenerator.cc	3	89.56%
pdns/recursordist/pdns_recursor.cc	4	72.57%
pdns/dnsdistdist/dnsdist-carbon.cc	6	63.51%
pdns/recursordist/syncres.cc	7	79.38%
pdns/recursordist/test-syncres_cc1.cc	8	89.58%

Totals
Change from base Build 12034691735:	-8.4%
Covered Lines:	125874
Relevant Lines:	163645

---

💛 - Coveralls

[rgacogne]

I see that reportBasicTypes() is only used by unit tests, reportOtherTypes() is no longer used after this PR, so I think they could be removed in favor of always calling reportAllTypes(). Once this is done I don't think anyone should call ::report() directly (the remote backend unit tests currently do, but it looks like it could call reportAllTypes() instead), so perhaps it would make sense to disallow that (for example by making it take a required parameter to a const-reference to an opaque object, that can only be constructed by reportAllTypes()). This way we can call DNSRecordContent::lock() from reportAllTypes() directly, preventing a mistake later where we call reportAllTypes() without calling lock().
Does that make sense to you? If so I can look into it if you want me to.

[rgacogne]

I pushed a commit implementing my proposal, please let me know if it looks good to you.

[omoerbeek]

I'm mostly happy with this. One remark: we tend to move to /* unsued */ instead of a void cast of the unused parameter. If you agree I can change that before merge.

[rgacogne]

I'm mostly happy with this. One remark: we tend to move to /* unused */ instead of a void cast of the unused parameter. If you agree I can change that before merge.

Sure!

[rgacogne]

You might want to change this one as well for consistency.