Pull request 1806: 5661-fix-protection-update-lock

Updates #5661.

Squashed commit of the following:

commit 02e83c75c8f44f084c0cb8d33b7d6a524c8c1b0e
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Apr 6 19:28:17 2023 +0300

    dnsforward: imp logs

commit 0f27265fc94f0e3b8e2dee79dbf9ab5344416c61
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Thu Apr 6 19:18:19 2023 +0300

    dnsforward: imp locks
This commit is contained in:
Ainar Garipov 2023-04-06 19:33:25 +03:00
parent b1120221c7
commit 5d5a729569
2 changed files with 36 additions and 15 deletions

View File

@ -587,11 +587,11 @@ func (s *Server) prepareTLS(proxyConfig *proxy.Config) (err error) {
if s.conf.StrictSNICheck {
if len(cert.DNSNames) != 0 {
s.conf.dnsNames = cert.DNSNames
log.Debug("dnsforward: using certificate's SAN as DNS names: %v", cert.DNSNames)
log.Debug("dns: using certificate's SAN as DNS names: %v", cert.DNSNames)
slices.Sort(s.conf.dnsNames)
} else {
s.conf.dnsNames = append(s.conf.dnsNames, cert.Subject.CommonName)
log.Debug("dnsforward: using certificate's CN as DNS name: %s", cert.Subject.CommonName)
log.Debug("dns: using certificate's CN as DNS name: %s", cert.Subject.CommonName)
}
}
@ -649,16 +649,8 @@ func (s *Server) onGetCertificate(ch *tls.ClientHelloInfo) (*tls.Certificate, er
// UpdatedProtectionStatus updates protection state, if the protection was
// disabled temporarily. Returns the updated state of protection.
func (s *Server) UpdatedProtectionStatus() (enabled bool) {
changed := false
defer func() {
if changed {
log.Info("dns: protection is restarted after pause")
s.conf.ConfigModified()
}
}()
s.serverLock.Lock()
defer s.serverLock.Unlock()
s.serverLock.RLock()
defer s.serverLock.RUnlock()
disabledUntil := s.conf.ProtectionDisabledUntil
if disabledUntil == nil {
@ -669,9 +661,33 @@ func (s *Server) UpdatedProtectionStatus() (enabled bool) {
return false
}
s.conf.ProtectionEnabled = true
s.conf.ProtectionDisabledUntil = nil
changed = true
// Update the values in a separate goroutine, unless an update is already in
// progress. Since this method is called very often, and this update is a
// relatively rare situation, do not lock s.serverLock for writing, as that
// can lead to freezes.
//
// See https://github.com/AdguardTeam/AdGuardHome/issues/5661.
if s.protectionUpdateInProgress.CompareAndSwap(false, true) {
go s.enableProtectionAfterPause()
}
return true
}
// enableProtectionAfterPause sets the protection configuration to enabled
// values. It is intended to be used as a goroutine.
func (s *Server) enableProtectionAfterPause() {
defer log.OnPanic("dns: enabling protection after pause")
defer s.protectionUpdateInProgress.Store(false)
defer s.conf.ConfigModified()
s.serverLock.Lock()
defer s.serverLock.Unlock()
s.conf.ProtectionEnabled = true
s.conf.ProtectionDisabledUntil = nil
log.Info("dns: protection is restarted after pause")
}

View File

@ -9,6 +9,7 @@ import (
"runtime"
"strings"
"sync"
"sync/atomic"
"time"
"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
@ -111,6 +112,10 @@ type Server struct {
isRunning bool
// protectionUpdateInProgress is used to make sure that only one goroutine
// updating the protection configuration after a pause is running at a time.
protectionUpdateInProgress atomic.Bool
conf ServerConfig
// serverLock protects Server.
serverLock sync.RWMutex