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.

Sign up for GitHub

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

Jump to bottom

Lock masquerade object during json.Marshal #41

Merged
merged 2 commits into from
Aug 12, 2024
Merged

Lock masquerade object during json.Marshal #41

merged 2 commits into from
Aug 12, 2024

Conversation

atavism
Copy link
Contributor

@atavism atavism commented Aug 12, 2024

@github-actions GitHub Actions
Copy link

Coverage Status

coverage: 84.592% (+0.3%) from 84.275%
when pulling 318e336 on atavism/cache-updates
into 9369ca3 on main.

WendelHime
WendelHime approved these changes Aug 12, 2024
View reviewed changes
Copy link
Contributor

@WendelHime WendelHime left a comment

Choose a reason for hiding this comment

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

Tested locally three times and no data race detected!

@atavism
Copy link
Contributor Author

atavism commented Aug 12, 2024

Thanks for the review, @WendelHime!!

hwh33
hwh33 approved these changes Aug 12, 2024
View reviewed changes
Copy link
Contributor

@hwh33 hwh33 left a comment

Choose a reason for hiding this comment

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

LGTM

masquerade.go
m.mx.RLock()
defer m.mx.RUnlock()
// Type alias for masquerade so that we don't infinitely recurse when marshaling the struct
type alias masquerade
Copy link
Contributor

Choose a reason for hiding this comment

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

Nifty!

@atavism
Copy link
Contributor Author

atavism commented Aug 12, 2024

Thanks, @hwh33!

@atavism atavism merged commit d549c37 into main Aug 12, 2024
2 checks passed
@atavism atavism deleted the atavism/cache-updates branch August 12, 2024 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WendelHime WendelHime WendelHime approved these changes

@hwh33 hwh33 hwh33 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@atavism @hwh33 @WendelHime