Fix silent channel loss from randomised filter evaluation order - #664
Open
slbailey wants to merge 1 commit into
Open
slbailey wants to merge 1 commit into
slbailey wants to merge 1 commit into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
filterThisStream() returned false as soon as any filter matched a stream
but rejected it on an {include} / !{exclude} condition. That ended the
whole loop, so the stream never reached the filter it actually belonged
to and was silently deactivated.
On its own that would be a deterministic bug. It is worse than that
because createFilterRules() builds Data.Filter by ranging over
Settings.Filter, which is a map - and Go randomises map iteration order.
Nothing sorts it afterwards, so filter evaluation order differs on every
buildDatabaseDVR() call and a channel can be active on one scan and gone
on the next with no configuration change at all.
The two combine badly with a custom-filter whose rule is only condition
groups, e.g. "{16,21,31}". The {...} group is stripped out to become the
include list, leaving filter.Rule empty - and strings.Contains(s, "") is
always true, so that filter matches every stream. Whenever it happens to
sort before another filter, every channel that does not satisfy its
include list is dropped.
Because cleanupXEPG() then permanently deletes any XEPG channel missing
from Data.Cache.Streams.Active, the loss is not transient: the mapping is
gone from xepg.json and the channel disappears from the Plex/Emby guide.
Fixes:
- filterThisStream(): continue instead of return false, so one filter's
condition can no longer veto the whole chain.
- createFilterRules(): sort the filter IDs before building Data.Filter so
evaluation order is deterministic and matches the UI order.
Verified against 6b9c0cc: go build ./... and a linked binary both fine,
and go vet / gofmt -l output is byte-identical to unpatched main.
Copy link
Copy Markdown
Author
|
One more data point, since branch-1.2.40 is the newest unreleased work in this repo and the obvious question is whether it already covers this: it does not. On branch-1.2.40 today, src/data.go:756 is still for _, f := range Settings.Filter over the map, both condition-failure paths in filterThisStream() are still return false, liveEvent, and the custom-filter case still does a bare strings.Contains(search, filter.Rule) with no guard for the empty rule left behind by brace extraction. So all three parts of this reproduce on 1.2.37, 1.2.38, 1.2.39, main and branch-1.2.40 alike. |
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
XEPG channel mappings disappear on their own, with no configuration change and no error in the log. I lost ErsatzTV channels out of the Plex guide repeatedly — sometimes one channel, sometimes seven — and it always looked nondeterministic. It is, and this is why.
Three things combine
1. filterThisStream() lets one filter veto every other filter. When a filter matches a stream but rejects it on an {include} / !{exclude} condition, the function return falses — ending the whole loop. The stream never reaches the filter it actually belongs to, and is silently deactivated.
2. Filter evaluation order is randomised on every scan. createFilterRules() builds Data.Filter with for _, f := range Settings.Filter, and Settings.Filter is a map[int64]interface{}. Go randomises map iteration order, and nothing sorts it afterwards. So which filter gets first claim on a stream changes from one buildDatabaseDVR() to the next — same config, same sources, different result.
3. A conditions-only custom-filter matches everything. A rule like {16,21,31} is entirely consumed by the include-extraction regex, leaving filter.Rule == "" — and strings.Contains(s, "") is always true in Go. That filter therefore matches every stream and falls straight through to its include check.
Put together: a conditions-only custom-filter that happens to sort before your other filters will claim every stream, reject the ones failing its include list, and — because of #1 — drop them entirely instead of letting their real filter have them.
The loss is permanent, not transient: cleanupXEPG() deletes any XEPG channel missing from Data.Cache.Streams.Active, so the mapping is erased from xepg.json and the channel vanishes from the guide. Nothing crashes, other channels keep working, and /lineup.json answers normally — just with fewer entries. It is very easy to miss.
Reproduction
Three filters — 24x7 (group-title), ErsatzTV (group-title), OTA (custom-filter, rule {16,21,31,32,33,47}) — over a fixed 118-stream playlist, 8 of them ErsatzTV. I ported parsePlaylist + filterThisStream and enumerated all six orderings:
Four of six orderings lose channels — a 67% chance per update cycle. Every one of these counts matched what I saw in production, including which specific channels went. One channel survived every single cull, which looked like a clue but was pure coincidence: its tvg-id contains 47, one of the OTA include keys, so it passed that filter's check no matter when it ran.
Worth stressing that the source was never at fault. I probed the upstream m3u every minute across the update window: HTTP 200, byte-identical, all 8 channels present. All streams: 118 is logged identically on culled and healthy cycles — only Active streams moves.
The change
Small enough to read in one sitting, and it leaves the "source no longer in settings" delete alone — that one is an explicit user action.
Verification
Built against 6b9c0cc in golang:1.22: go build -mod=mod ./... passes and the binary links (14 MB). go vet and gofmt -l output is byte-identical to unpatched main — 6 and 11 pre-existing findings respectively, none introduced here. (Note -mod=mod is required because vendor/modules.txt is out of sync with go.mod on main; unrelated to this change.)
Workaround without patching
If you are hitting this and cannot run a patched build, give the conditions-only custom-filter a base rule that its own conditions will match, so it stops matching every stream. In my case adding a token common to the affected channels to the include list was enough — verified to produce an identical active set across all six orderings.
Relation to #663
#663 adds a consecutive-miss counter to cleanupXEPG(). I filed it before I had the real cause and it treats the symptom — worth keeping as defence in depth, since it also covers a genuinely flaky source, but this PR is the actual fix. Happy to close #663, rebase either one, or split this into two commits — whatever suits.