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 issue120 and issue103: add mutex to prevent data race in websocket write message and use named return values for WritePkg method #121

Merged
merged 8 commits into from
May 28, 2024

Conversation

No-SilverBullet
Copy link

@No-SilverBullet No-SilverBullet commented May 20, 2024

What this PR does:

based on doc: https://pkg.go.dev/github.com/gorilla/websocket#hdr-Concurrency and #120

1.add lock (sync.Mutex) in gettyWSConn struct
2. add new function 'threadSafeWriteMessage' to ensure that only one thread can send a message at a time,preventing race conditions.

based on #103
3. use named return values for WritePkg method to return the captured panic to caller, which can prevent the panic in caller.
Which issue(s) this PR fixes:

Fixes #120, #103

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

@FoghostCn FoghostCn requested a review from AlexStocks May 20, 2024 12:42
Copy link

@FoghostCn FoghostCn left a comment

Choose a reason for hiding this comment

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

Should we use the packetLock in the session?

@AlexStocks
Copy link

AlexStocks commented May 20, 2024

@No-SilverBullet good job. Maybe u can also check the tcp write is thread safe or not. refer issue #103

@No-SilverBullet
Copy link
Author

Should we use the packetLock in the session?

Based on this issue120 and document( https://pkg.go.dev/github.com/gorilla/websocket#hdr-Concurrency

), the writeMessage function in gorrila package is not concurrency safe, so a lock is added to prevent race conditions.

@No-SilverBullet
Copy link
Author

@No-SilverBullet good job. Maybe u can also check the tcp write is thread safe or not. refer issue #103

ok

No-SilverBullet and others added 2 commits May 24, 2024 13:51
refactor:use named return values for WritePkg method to return the ca…
@No-SilverBullet No-SilverBullet changed the title fix issue120: add mutex to prevent data race in websocket write message fix issue120 and issue103: add mutex to prevent data race in websocket write message and use named return values for WritePkg method May 24, 2024
@No-SilverBullet
Copy link
Author

Should we use the packetLock in the session?

Based on this issue120 and document( https://pkg.go.dev/github.com/gorilla/websocket#hdr-Concurrency

), the writeMessage function in gorrila package is not concurrency safe, so a lock is added to prevent race conditions.

session.go Outdated Show resolved Hide resolved
@AlexStocks AlexStocks merged commit 2769505 into apache:master May 28, 2024
1 check passed
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.

Concurrency issues can occur when using this framework for websocket transfers
4 participants