[code sync] Merge code from sonic-net/sonic-host-services:202605 to 202608 - #17
Merged
Merged
Conversation
mssonicbld
commented
Jul 31, 2026
mssonicbld
commented
Collaborator
…ninstall (#411)
* featured: fix stale FEATURE entry after concurrent package uninstall
When sonic-package-manager uninstalls a package it deletes the FEATURE
entry from CONFIG_DB. Because featured unblocks spm (via STATE_DB) before
finishing its own handler, the entry can be gone by the time featured tries
to write back to it. Two code paths independently recreate the deleted
entry, leaving a ghost record that survives the uninstall and appears in
"show feature status":
1. sync_feature_scope(): called after a successful disable_feature(), it
calls mod_entry({has_per_asic_scope, has_global_scope}) on the already-
deleted key. Redis HSET recreates the hash with only those two fields
(no 'state'), so the next notification arrives with state=None and
featured logs "ERR: Unexpected state value 'None'".
There is also a TOCTOU gap: spm can delete the entry between the
mod_entry calls and the post-write read, producing the same partial
entry.
2. resync_feature_state(): called when update_feature_state() fails (e.g.
systemctl start fails because the service file was already removed by
the concurrent uninstall). get_entry() returns {} for a deleted key,
_feature_state_is_template(None) evaluates True, and mod_entry writes
{state: disabled} back - recreating the entry with no other fields.
The same function also completes partial entries left by (1): a non-
empty entry with no 'state' key passes the deleted-entry guard and
receives a state field, producing a 3-field ghost record.
Fix all paths:
- sync_feature_scope: after the two mod_entry writes, re-read the entry.
If it exists but has no 'state' key, the write raced with deregister;
delete the partial entry immediately.
- resync_feature_state: return early if the entry is fully absent. If
the entry exists but has no 'state' key, delete it rather than
completing the ghost record.
- update_feature_state: suppress the ERR log when state is None, since
that is a normal consequence of the race rather than a programming error.
Signed-off-by: david.zagury <davidza@nvidia.com>
* featured: update and extend tests for deregister race guards
test_feature_resync: the sub-case where get_entry returns None previously
asserted that mod_entry would be called with the rendered state. That
behaviour was the root of the bug -- resync_feature_state was recreating a
FEATURE row that had been deleted by a concurrent deregister. Update the
assertion to assert_not_called(), matching the new guard that returns early
when the entry is absent. Add a further sub-case for a partial entry (row
exists but has no 'state' key, the TOCTOU artifact left by sync_feature_scope
racing with deregister): resync_feature_state must call set_entry(None) to
remove the ghost record rather than completing it with mod_entry.
test_sync_feature_scope_toctou: new test for the post-write TOCTOU guard in
sync_feature_scope. Uses side_effect to return a live entry on the pre-write
check and a partial entry (no 'state' key) on the post-write check, simulating
the window where the FEATURE row is deleted and recreated by mod_entry during
the race. sync_feature_scope must call set_entry(None) to clean it up.
Signed-off-by: david.zagury <davidza@nvidia.com>
* Include 'state' TOCTOU cleanup does not treat this as a partial ghost entry.
Signed-off-by: david.zagury <davidza@nvidia.com>
* Potential fix for pull request finding
Signed-off-by: david.zagury <davidza@nvidia.com>
---------
Signed-off-by: david.zagury <davidza@nvidia.com>
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
2 participants
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.