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

Fix possible race conditions when suspending/unsuspending accounts #22363

Conversation

ClearlyClaire
Copy link
Contributor

Currently, when suspending or unsuspending an account, the suspended_at flag is immediately updated, and the heavy work is performed in a background job. However, the background job sets the flag again (losing suspension origin in the process!) if it isn't set, which can lead to surprising and unwanted results: if an account is unsuspended before the suspension worker runs:

  • in the best case scenario, the account will be suspended then promptly unsuspended by the unsuspension worker, the long-time result is not an issue, but there will be extra work and a time window in which the account will be marked as suspended for no good reason
  • in the case where the suspension and unsuspension worker are picked up by different processes at the same time, the result is undefined
  • in cases where the suspension or unsuspension workers fail for some reason, they will re-set the flag at each attempt, causing a highly confusing behavior (account re-suspended even though admins unsuspended the account, multiple times)
  • it can change a remote suspension flag into a local suspension flag, which is very confusing

Therefore, this PR changes the suspension/unsuspension worker logics to not set the suspension flag, but check it instead: the appropriate worker will only be scheduled after the flag has been set appropriately on the call site, so this should be safe.

Tests were assuming SuspensionWorker and UnsuspensionWorker would do the
suspending/unsuspending themselves, but this has changed.
@Gargron Gargron merged commit 18fb01e into mastodon:main Jan 5, 2023
nametoolong pushed a commit to nametoolong/nuage that referenced this pull request Jan 12, 2023
…astodon#22363)

* Fix possible race conditions when suspending/unsuspending accounts

* Fix tests

Tests were assuming SuspensionWorker and UnsuspensionWorker would do the
suspending/unsuspending themselves, but this has changed.
ClearlyClaire added a commit to ClearlyClaire/mastodon that referenced this pull request Feb 9, 2023
…astodon#22363)

* Fix possible race conditions when suspending/unsuspending accounts

* Fix tests

Tests were assuming SuspensionWorker and UnsuspensionWorker would do the
suspending/unsuspending themselves, but this has changed.
ClearlyClaire added a commit that referenced this pull request Feb 9, 2023
…22363) (#23482)

* Fix possible race conditions when suspending/unsuspending accounts

* Fix tests

Tests were assuming SuspensionWorker and UnsuspensionWorker would do the
suspending/unsuspending themselves, but this has changed.
atsu1125 pushed a commit to atsu1125/mastodon that referenced this pull request Feb 17, 2023
…astodon#22363) (mastodon#23482)

* Fix possible race conditions when suspending/unsuspending accounts

* Fix tests

Tests were assuming SuspensionWorker and UnsuspensionWorker would do the
suspending/unsuspending themselves, but this has changed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants