fix: parental-control switch left stale on rule removal (cache_time race + missing else-branch)#1255
Open
paul43210 wants to merge 1 commit into
Open
Conversation
Two related bugs cause `switch.<client>_block_internet` entities to remain `state=on` after their underlying parental-control rule is removed: 1. `_get_data_parental_control` defaults to `force=False`, so the post-write refresh in `async_service_device_internet_access` receives cached pre-write data when called within `cache_time=5s` of the previous poll. The subsequent platform unload+reload then iterates the stale `pc_rules` and re-creates the switch for a rule that no longer exists on the router. 2. `ClientInternetSwitch.async_on_demand_update` has no else-branch. Once `pc_rules` catches up and the MAC is gone, the switch's cached `_rule` (with `type=BLOCK`) is never invalidated. `is_on` keeps returning True for as long as the entity exists. Fixes: - Thread `force: bool = False` through `_get_data_parental_control` and `update_pc_rules`; pass `force=True` from the service handler so the post-write fetch bypasses the cache. - Add an else-branch to `async_on_demand_update` that marks the entity unavailable when its MAC is gone from `pc_rules`. Covers out-of-band removals too (router UI, ASUS app, 64-rule cap eviction). Verified against ZenWiFi BT8 firmware 3.0.0.6.102_58407 with 100+ block/unblock cycles: zero zombie switches.
Author
|
Heads-up before review: in the interest of being honest, this PR was largely AI-drafted (Claude, Anthropic) — the code transform, tests, and prose are AI-generated. The bug investigation and validation against my hardware (ZenWiFi BT8 mesh on FW Happy to elaborate on the diagnosis or the change, and equally happy to take rework feedback or close if it's not a fit. |
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
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.
Summary
Two related bugs cause
switch.<client>_block_internetentities to remainstate=onafter their underlying parental-control rule is removed (via thedevice_internet_access state=removeservice, the router admin UI, the ASUS app, or 64-rule cap eviction). The switch becomes a zombie that misreports the device as blocked indefinitely, until the user manually reloads the integration.Observed against ZenWiFi BT8 firmware
3.0.0.6.102_58407with a Node-RED-driven workflow that frequently issuesdevice_internet_accesscalls. Reproduced multiple times in a single debugging session; confirmed fully resolved after the patch with 100+ block/unblock cycles producing zero zombies.Root cause
Bug 1 —
bridge.py:_get_data_parental_controldoesn't honour write-then-read freshnessCONF_DEFAULT_CACHE_TIME = 5seconds. Whenasync_service_device_internet_accessinrouter.pycallsupdate_pc_rules()immediately after a successfulasync_pc_rule(REMOVE), the library returns its still-cached pre-write rule list.self._pc_rulesis then refreshed from stale data — the just-removed MAC is still present. The subsequent platform unload+reload (also in the service handler) iterates this stalepc_rulesand re-createsClientInternetSwitchfor a rule that no longer exists on the router.is_oncontinues to returnTruebased on the cached_rule.type=BLOCK.Bug 2 —
switch.py:ClientInternetSwitch.async_on_demand_updatehas no else-branchOnce
update_pc_ruleseventually catches up (next poll cycle, cache expired),signal_pc_rules_updatefires andasync_on_demand_updateruns for every existing switch. For the just-removed MAC, theiftest fails and the function silently returns. The cachedself._ruleis never invalidated,is_onkeeps returningTrue. Subsequent polls continue to no-op for as long as the entity exists — only an integration reload clears it.Why both fixes
Either fix in isolation would address the immediate symptom for service-triggered removes (since the service handler already does a platform unload+reload). Both fixes are needed because:
Reproduction
asusrouter.device_internet_accesswithstate=blockfor a fresh MAC. Confirm the rule appears in the router admin UI andswitch.<client>_block_internetbecomesstate=on.state=remove.switch.<client>_block_internetremainsstate=onandis_on=True.unavailableor disappears, confirming the cached state was the issue.Verification after patch
Same reproduction; the switch correctly drops to
unavailableimmediately after the remove, and the entity is cleanly removed by the platform reload. Tested with 100+ block/unblock cycles in a single session: zero zombie switches.Test plan
🤖 Generated with Claude Code