miekg / dns

DNS library in Go
https://miek.nl/2014/august/16/go-dns-package
BSD 3-Clause "New" or "Revised" License
8.09k stars 1.14k forks source link

A potential goroutine leak in server_test.go #1610

Open xuxiaofan1203 opened 1 month ago

xuxiaofan1203 commented 1 month ago

For the lock and condition variable, if the testShutdownNotify.Wait() get executed very late, some goroutine will leak. https://github.com/miekg/dns/blob/b77d1ed8e9282cadf21c4124f53a660fed55c8ca/server_test.go#L733-L735 The interleaved sequence that trigger bugs can be reproduced by adding time.Sleep to make Wait() executes late like this:

testShutdownNotify.L.Lock()
time.Sleep(time.Second)
testShutdownNotify.Wait()
testShutdownNotify.L.Unlock()

After that, you can use goleak to reproduce the bug in the test function. https://github.com/miekg/dns/blob/b77d1ed8e9282cadf21c4124f53a660fed55c8ca/server_test.go#L825 Some outputs about the bug from goleak are as follows:

Goroutine 85 in state sync.Cond.Wait, with sync.runtime_notifyListWait on top of the stack:
        sync.runtime_notifyListWait(0xc00007e1d0, 0x0)
            /usr/local/go/src/runtime/sema.go:587 +0x159
        sync.(*Cond).Wait(0x3b9aca00?)
            /usr/local/go/src/sync/cond.go:71 +0x85
        github.com/miekg/dns.checkInProgressQueriesAtShutdownServer.func2({0x8b3e30, 0xc000038c80}, 0xc0000f54d0)
            /home/song2048/桌面/goProject/src/github.com/system-pclub/GCatch/GCatch/testdata/src/github.com/dns/server_test.go:736 +0x9a
miekg commented 5 days ago

... in the test functions?? Do you have an PR?

xuxiaofan1203 commented 5 days ago

... in the test functions?? Do you have an PR?

Yes, the bug is in the test functions mentioned above. I found the bug, but I'm not sure how to fix it. So I haven't submitted an PR.