diff --git a/internal/filtering/filter.go b/internal/filtering/filter.go index e64c2f12..44dc7f76 100644 --- a/internal/filtering/filter.go +++ b/internal/filtering/filter.go @@ -176,13 +176,16 @@ func (d *DNSFilter) filterExistsLocked(url string) (ok bool) { // Add a filter // Return FALSE if a filter with this URL exists -func (d *DNSFilter) filterAdd(flt FilterYAML) bool { +func (d *DNSFilter) filterAdd(flt FilterYAML) (err error) { + // Defer annotating to unlock sooner. + defer func() { err = errors.Annotate(err, "adding filter: %w") }() + d.filtersMu.Lock() defer d.filtersMu.Unlock() - // Check for duplicates + // Check for duplicates. if d.filterExistsLocked(flt.URL) { - return false + return errFilterExists } if flt.white { @@ -190,7 +193,8 @@ func (d *DNSFilter) filterAdd(flt FilterYAML) bool { } else { d.Filters = append(d.Filters, flt) } - return true + + return nil } // Load filters from the disk @@ -238,6 +242,7 @@ func updateUniqueFilterID(filters []FilterYAML) { } } +// TODO(e.burkov): Improve this inexhaustible source of races. func assignUniqueFilterID() int64 { value := nextFilterID nextFilterID++ @@ -343,29 +348,31 @@ func (d *DNSFilter) refreshFiltersArray(filters *[]FilterYAML, force bool) (int, } updateCount := 0 + + d.filtersMu.Lock() + defer d.filtersMu.Unlock() + for i := range updateFilters { uf := &updateFilters[i] updated := updateFlags[i] - d.filtersMu.Lock() for k := range *filters { f := &(*filters)[k] if f.ID != uf.ID || f.URL != uf.URL { continue } + f.LastUpdated = uf.LastUpdated if !updated { continue } - log.Info("Updated filter #%d. Rules: %d -> %d", - f.ID, f.RulesCount, uf.RulesCount) + log.Info("Updated filter #%d. Rules: %d -> %d", f.ID, f.RulesCount, uf.RulesCount) f.Name = uf.Name f.RulesCount = uf.RulesCount f.checksum = uf.checksum updateCount++ } - d.filtersMu.Unlock() } return updateCount, updateFilters, updateFlags, false @@ -421,11 +428,16 @@ func (d *DNSFilter) refreshFiltersIntl(block, allow, force bool) (int, bool) { if !updated { continue } - _ = os.Remove(uf.Path(d.DataDir) + ".old") + + p := uf.Path(d.DataDir) + err := os.Remove(p + ".old") + if err != nil { + log.Debug("filtering: removing old filter file %q: %s", p, err) + } } } - log.Debug("filtering: update finished") + log.Debug("filtering: update finished: %d lists updated", updNum) return updNum, false } @@ -467,8 +479,8 @@ func scanLinesWithBreak(data []byte, atEOF bool) (advance int, token []byte, err } // parseFilter copies filter's content from src to dst and returns the number of -// rules, name, number of bytes written, checksum, and title of the parsed list. -// dst must not be nil. +// rules, number of bytes written, checksum, and title of the parsed list. dst +// must not be nil. func (d *DNSFilter) parseFilter( src io.Reader, dst io.Writer, @@ -550,14 +562,18 @@ func isHTML(line string) (ok bool) { return strings.HasPrefix(line, "