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

make secrets.Store middleware threadsafe #625

Merged
merged 8 commits into from
Jul 27, 2023

Conversation

pacejackson
Copy link
Contributor

@pacejackson pacejackson commented Jul 27, 2023

💸 TL;DR

Guard secrets.Store middleware with a mutex to make adding new middlewares to an existing store threadsafe.

📜 Details

Sometimes we have to call AddMiddleware in a goroutine which can lead to race conditions with other calls to AddMiddleware. Adding a mutex around updating and calling the middleware functions protects us from this.

✅ Checks

  • CI tests (if present) are passing
  • Adheres to code style for repo

@pacejackson pacejackson marked this pull request as ready for review July 27, 2023 17:10
@pacejackson pacejackson requested a review from a team as a code owner July 27, 2023 17:10
@pacejackson pacejackson requested review from fishy, kylelemons and konradreiche and removed request for a team July 27, 2023 17:10
@@ -4,7 +4,6 @@ import (
"bytes"
"context"
"encoding/json"

Copy link
Member

Choose a reason for hiding this comment

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

nit: don't remove this blank line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird... not sure where that came from

@pacejackson pacejackson requested a review from fishy July 27, 2023 17:21
@pacejackson pacejackson merged commit 8b80dad into reddit:master Jul 27, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants