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

Improve LMDBBackend reliability #15175

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

miodvallat
Copy link
Contributor

Short description

Recent tinkering made me discover the hard way that, in some corner (error) cases, I could get a double free in the LMDBBackend destructor.

Further investigation showed that we can end up with two active LMDB transactions; these need to be aborted in the reverse order of their creation, and, well, this wasn't always the case.

This PR fixes this problem.

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)

If the LMDBBackend destructor is invoked while there are still pending
transactions, these need to be aborted, but in the reverse order of
their creation (i.e. abort the innermost transaction first).

The default destructor would abort them in a class field
declaration-dependent order, which may not match the actual cinematic.

We now remember which transaction is the innermost one, so that we can
abort them in the expected order.

This gets rid of "double free or corruption (top)" aborts with glibc,
and Address Sanitizer errors.
@coveralls
Copy link

Pull Request Test Coverage Report for Build 13409190143

Details

  • 13 of 13 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1974 unchanged lines in 59 files lost coverage.
  • Overall coverage decreased (-1.0%) to 63.523%

Files with Coverage Reduction New Missed Lines %
ext/json11/json11.cpp 1 64.49%
ext/json11/json11.hpp 1 46.67%
pdns/auth-zonecache.hh 1 86.67%
pdns/dnsbackend.hh 1 55.04%
pdns/auth-packetcache.hh 2 90.0%
pdns/auth-querycache.hh 2 85.71%
pdns/dnsdistdist/dnsdist-backend.cc 2 66.08%
pdns/recursordist/lwres.cc 2 69.43%
ext/yahttp/yahttp/url.hpp 3 53.98%
pdns/dnsdistdist/dnsdist-doh-common.hh 3 84.91%
Totals Coverage Status
Change from base Build 13374437384: -1.0%
Covered Lines: 123465
Relevant Lines: 163802

💛 - Coveralls

@miodvallat miodvallat merged commit 72b4db3 into PowerDNS:master Feb 19, 2025
83 checks passed
@miodvallat miodvallat deleted the lmdb_abort_with_style branch February 19, 2025 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants