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

[client] Fix race condition while read/write conn status in peer conn #2607

Merged
merged 2 commits into from
Sep 17, 2024

Conversation

pappz
Copy link
Contributor

@pappz pappz commented Sep 16, 2024

Describe your changes

The ConnStatus variable has been read in internal/peer/conn.go:330 and has been written in internal/peer/conn.go:422. It was not thread safe.

Issue ticket number and link

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • Extended the README / documentation, if necessary

Copy link

sonarcloud bot commented Sep 16, 2024

@pappz pappz marked this pull request as ready for review September 16, 2024 13:48
@pappz pappz changed the title Fix race condition Fix race condition while read/write conn status in peer conn Sep 17, 2024

// AtomicConnStatus is a thread-safe wrapper for ConnStatus
type AtomicConnStatus struct {
status atomic.Int32
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could remove the wrapper as it's only a single field, but that's a minor thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did the wrapper because of the atom.Int32 stores Int32 and the ConnStatus is a custom type (because of the String method). So I tried to avoid the cast steps.

Copy link
Contributor

@lixmal lixmal Sep 17, 2024

Choose a reason for hiding this comment

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

I meant doing type AtomicConnStatus atomic.Int32 instead of the struct. But it's all right

@mlsmaycon mlsmaycon changed the title Fix race condition while read/write conn status in peer conn [client] Fix race condition while read/write conn status in peer conn Sep 17, 2024
@mlsmaycon mlsmaycon merged commit 1104c9c into main Sep 17, 2024
21 checks passed
@mlsmaycon mlsmaycon deleted the fix/status-race-condition branch September 17, 2024 09:15
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.

3 participants