Merge pull request #1955 from safing/fix/1951-Filter-list-update-failed

fix(filterlists): Resolve occasional FilterLists initialization failures
This commit is contained in:
Alexandr Stelnykovych
2025-08-05 16:30:51 +03:00
committed by GitHub
5 changed files with 41 additions and 5 deletions

View File

@@ -110,6 +110,7 @@ func processListFile(ctx context.Context, filter *scopedBloom, file *updates.Art
}) })
} }
// Decode file and send entries to values channel.
startSafe(func() (err error) { startSafe(func() (err error) {
defer close(values) defer close(values)
@@ -117,6 +118,15 @@ func processListFile(ctx context.Context, filter *scopedBloom, file *updates.Art
return return
}) })
// Wait for the module initialization to complete to ensure the filter is fully loaded.
// This avoids prolonged locks during filter updates caused by concurrent database loading.
select {
case <-moduleInitDone:
case <-time.After(time.Second * 20):
log.Warning("intel/filterlists: timeout waiting for module initialization")
}
// Process each entry and send records to records channel.
startSafe(func() error { startSafe(func() error {
defer close(records) defer close(records)
for entry := range values { for entry := range values {
@@ -128,6 +138,7 @@ func processListFile(ctx context.Context, filter *scopedBloom, file *updates.Art
return nil return nil
}) })
// Persist records to the database.
persistRecords(startSafe, records) persistRecords(startSafe, records)
return g.Wait() return g.Wait()

View File

@@ -7,6 +7,7 @@ import (
"strings" "strings"
"sync" "sync"
"github.com/hashicorp/go-version"
"github.com/safing/portmaster/base/database" "github.com/safing/portmaster/base/database"
"github.com/safing/portmaster/base/database/record" "github.com/safing/portmaster/base/database/record"
"github.com/safing/portmaster/base/log" "github.com/safing/portmaster/base/log"
@@ -167,6 +168,16 @@ var (
listIndexUpdateLock sync.Mutex listIndexUpdateLock sync.Mutex
) )
// Compares two version strings and returns true only if both are successfully parsed and equal
func areSemversEqual(v1, v2 string) bool {
parsedV1, err1 := version.NewSemver(v1)
parsedV2, err2 := version.NewSemver(v2)
if err1 != nil || err2 != nil {
return false
}
return parsedV1.Equal(parsedV2)
}
func updateListIndex() error { func updateListIndex() error {
listIndexUpdateLock.Lock() listIndexUpdateLock.Lock()
defer listIndexUpdateLock.Unlock() defer listIndexUpdateLock.Unlock()
@@ -188,7 +199,9 @@ func updateListIndex() error {
log.Info("filterlists: index not in cache, starting update") log.Info("filterlists: index not in cache, starting update")
case err != nil: case err != nil:
log.Warningf("filterlists: failed to load index from cache, starting update: %s", err) log.Warningf("filterlists: failed to load index from cache, starting update: %s", err)
case listIndexUpdate.Version != strings.TrimPrefix(index.Version, "v"): case listIndexUpdate.Version != strings.TrimPrefix(index.Version, "v") &&
// Avoid false positives by checking if the version is actually different (e.g. "2025.04.14 == 2025.4.14")
!areSemversEqual(listIndexUpdate.Version, index.Version):
log.Infof( log.Infof(
"filterlists: index from cache is outdated, starting update (%s != %s)", "filterlists: index from cache is outdated, starting update (%s != %s)",
strings.TrimPrefix(index.Version, "v"), strings.TrimPrefix(index.Version, "v"),
@@ -196,7 +209,7 @@ func updateListIndex() error {
) )
default: default:
// List is in cache and current, there is nothing to do. // List is in cache and current, there is nothing to do.
log.Debug("filterlists: index is up to date") log.Debugf("filterlists: index is up to date (%s == %s)", index.Version, listIndexUpdate.Version)
// Update the unbreak filter list IDs on initial load. // Update the unbreak filter list IDs on initial load.
updateUnbreakFilterListIDs() updateUnbreakFilterListIDs()

View File

@@ -45,14 +45,17 @@ func (fl *FilterLists) Stop() error {
return stop() return stop()
} }
// booleans mainly used to decouple the module
// during testing.
var ( var (
moduleInitDone chan struct{}
// booleans mainly used to decouple the module during testing.
ignoreUpdateEvents = abool.New() ignoreUpdateEvents = abool.New()
ignoreNetEnvEvents = abool.New() ignoreNetEnvEvents = abool.New()
) )
func init() { func init() {
moduleInitDone = make(chan struct{})
ignoreNetEnvEvents.Set() ignoreNetEnvEvents.Set()
} }
@@ -87,6 +90,10 @@ func start() error {
filterListLock.Lock() filterListLock.Lock()
defer filterListLock.Unlock() defer filterListLock.Unlock()
// Signal that the module has been initialized.
// This indicates that the module is ready for use, with the default filter
defer close(moduleInitDone)
ver, err := getCacheDatabaseVersion() ver, err := getCacheDatabaseVersion()
if err == nil { if err == nil {
log.Debugf("intel/filterlists: cache database has version %s", ver.String()) log.Debugf("intel/filterlists: cache database has version %s", ver.String())
@@ -108,6 +115,7 @@ func start() error {
} }
func stop() error { func stop() error {
moduleInitDone = make(chan struct{})
filterListsLoaded = make(chan struct{}) filterListsLoaded = make(chan struct{})
return nil return nil
} }

View File

@@ -238,7 +238,7 @@ func getUpgradableFiles() ([]*updates.Artifact, error) {
func resolveUpdateOrder(updateOrder []*updates.Artifact) ([]*updates.Artifact, error) { func resolveUpdateOrder(updateOrder []*updates.Artifact) ([]*updates.Artifact, error) {
// sort the update order by ascending version // sort the update order by ascending version
sort.Sort(byAscVersion(updateOrder)) sort.Sort(byAscVersion(updateOrder))
log.Tracef("intel/filterlists: order of updates: %v", updateOrder) log.Tracef("intel/filterlists: order of potential updates: %v", updateOrder)
var cacheDBVersion *version.Version var cacheDBVersion *version.Version
if !isLoaded() { if !isLoaded() {

View File

@@ -59,6 +59,10 @@ func (a *Artifact) SemVer() *semver.Version {
return a.versionNum return a.versionNum
} }
func (a *Artifact) String() string {
return fmt.Sprintf("%s(v%s)", a.Filename, a.Version)
}
// IsNewerThan returns whether the artifact is newer than the given artifact. // IsNewerThan returns whether the artifact is newer than the given artifact.
// Returns true if the given artifact is nil. // Returns true if the given artifact is nil.
// The second return value "ok" is false when version could not be compared. // The second return value "ok" is false when version could not be compared.