coredns/plugin/route53/lock_repro_test.go

package route53

import (
	"context"
	"sync"
	"testing"
	"time"

	"github.com/coredns/coredns/core/dnsserver"
	"github.com/coredns/coredns/plugin"
	"github.com/coredns/coredns/plugin/file"
	"github.com/coredns/coredns/plugin/pkg/dnstest"
	"github.com/coredns/coredns/plugin/pkg/fall"
	"github.com/coredns/coredns/plugin/pkg/upstream"
	"github.com/coredns/coredns/plugin/test"

	"github.com/miekg/dns"
)

// TestServeDNSDoesNotHoldLockDuringUpstreamLookup reproduces the same bug
// class that plugin/azure fixed in PR #8447 ("plugin/azure: don't hold zMu
// across zone Lookup"), but for plugin/route53, where it is still present
// in route53.go's ServeDNS:
//
//	h.zMu.RLock()
//	m.Answer, m.Ns, m.Extra, result = hostedZone.z.Lookup(ctx, state, qname)
//	h.zMu.RUnlock()
//
// route53 zones are created with newZ.Upstream = h.upstream (see
// updateZones), so Lookup can chase a CNAME whose target is outside the
// zone via Upstream.Lookup, which re-enters the plugin chain and can block
// on real network I/O -- all while h.zMu.RLock() is held. Go's
// sync.RWMutex blocks new RLock() calls once a Lock() is pending, so one
// slow/hanging upstream lookup here stalls updateZones' periodic
// h.zMu.Lock() call, and every other query queues up behind that pending
// writer -- even for unrelated zones. This is also the direct ancestor of
// the still-open github.com/coredns/coredns/issues/6664: because Lookup
// runs *inside* the locked section, a panic inside it is caught by
// CoreDNS's per-request recover() in core/dnsserver/server.go but never
// reaches h.zMu.RUnlock(), permanently wedging the mutex.
func TestServeDNSDoesNotHoldLockDuringUpstreamLookup(t *testing.T) {
	const timeout = 5 * time.Second

	entered := make(chan struct{})
	release := make(chan struct{})
	var releaseOnce sync.Once
	releaseHandler := func() { releaseOnce.Do(func() { close(release) }) }
	defer releaseHandler()

	cfg := &dnsserver.Config{
		Zone: ".",
		Plugin: []plugin.Plugin{
			func(plugin.Handler) plugin.Handler {
				return plugin.HandlerFunc(func(_ context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
					close(entered)
					<-release
					m := new(dns.Msg)
					m.SetReply(r)
					m.Answer = []dns.RR{test.A("external.target. 300 IN A 5.6.7.8")}
					w.WriteMsg(m)
					return dns.RcodeSuccess, nil
				})
			},
		},
	}
	srv, err := dnsserver.NewServer("", []*dnsserver.Config{cfg})
	if err != nil {
		t.Fatal(err)
	}
	ctx := context.WithValue(context.Background(), dnsserver.Key{}, srv)

	newZ := file.NewZone("example.org.", "")
	newZ.Upstream = upstream.New()
	for _, rr := range []string{
		"example.org.	300	IN	SOA	ns1.example.org. hostmaster.example.org. 1 3600 300 2419200 300",
		"cname-ext.example.org. 300 IN CNAME external.target.",
	} {
		r, _ := dns.NewRR(rr)
		newZ.Insert(r)
	}

	h := &Route53{
		Fall:      fall.Zero,
		zoneNames: []string{"example.org."},
		zones:     zones{"example.org.": {{id: "Z1", dns: "example.org.", z: newZ}}},
	}

	req := new(dns.Msg)
	req.SetQuestion("cname-ext.example.org.", dns.TypeA)

	done := make(chan struct{})
	go func() {
		rec := dnstest.NewRecorder(&test.ResponseWriter{})
		h.ServeDNS(ctx, rec, req)
		close(done)
	}()

	select {
	case <-entered:
		// The query reached the upstream handler and is now blocked on
		// release, with h.zMu.RLock() (claimed) held for the duration if
		// the bug is present.
	case <-time.After(timeout):
		t.Fatal("query never reached the upstream handler")
	}

	lockAcquired := make(chan struct{})
	go func() {
		// Stands in for the periodic zone swap updateZones performs.
		h.zMu.Lock()
		h.zones["example.org."][0].z = file.NewZone("example.org.", "")
		h.zMu.Unlock()
		close(lockAcquired)
	}()

	select {
	case <-lockAcquired:
		// h.zMu.Lock() was not blocked by the in-flight upstream lookup:
		// the fix is in place.
	case <-time.After(timeout):
		t.Fatal("h.zMu.Lock() blocked while a query awaited a slow upstream lookup (bug reproduced)")
	}

	releaseHandler()

	select {
	case <-done:
	case <-time.After(timeout):
		t.Fatal("ServeDNS did not complete after the upstream handler was released")
	}
}