Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Free current client asynchronously after user permissions changes #13274

Conversation

valentinogeron
Copy link
Contributor

The crash happens when the user that triggers the permission changes should be affected (and should be disconnected eventually).

To handle such a scenario, we should use the CLIENT_CLOSE_AFTER_COMMAND flag.

This commit encapsulates all the places that should be handled in the same way in deauthenticateAndCloseClient

Also:

  • bugfix: during the ACL LOAD we ignore clients that are marked as CLIENT MASTER

The crash happens when the user that triggers the permission changes should be affected (and should be disconnected eventually).

To handle such a scenario, we should use the `CLIENT_CLOSE_AFTER_COMMAND` flag.

This commit encapsulates all the places that should be handled in the same way in `deauthenticateAndCloseClient`

Also:
* bugfix: during the ACL LOAD we ignore clients that are marked as `CLIENT MASTER`
@sundb sundb added the release-notes indication that this issue needs to be mentioned in the release notes label May 22, 2024
Copy link
Collaborator

@sundb sundb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@valentinogeron valentinogeron changed the title fix client disconnection after user permissions changes Free current client asynchronously after user permissions changes May 30, 2024
@sundb sundb merged commit 50569a9 into redis:unstable May 30, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes indication that this issue needs to be mentioned in the release notes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants