apache / dubbo-getty

a netty like asynchronous network I/O library based on tcp/udp/websocket; a bidirectional RPC framework based on JSON/Protobuf; a microservice framework based on zookeeper/etcd
Apache License 2.0
220 stars 69 forks source link

fix issue120 and issue103: add mutex to prevent data race in websocket write message and use named return values for WritePkg method #121

Closed No-SilverBullet closed 4 months ago

No-SilverBullet commented 4 months ago

What this PR does:

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

1.add lock (sync.Mutex) in gettyWSConn struct

  1. add new function 'threadSafeWriteMessage' to ensure that only one thread can send a message at a time,preventing race conditions.

based on https://github.com/apache/dubbo-getty/issues/103

  1. 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 https://github.com/apache/dubbo-getty/issues/120, https://github.com/apache/dubbo-getty/issues/103

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE
AlexStocks commented 4 months ago

@No-SilverBullet good job. Maybe u can also check the tcp write is thread safe or not. refer issue https://github.com/apache/dubbo-getty/issues/103

No-SilverBullet commented 4 months ago

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 commented 4 months ago

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

ok

No-SilverBullet commented 4 months ago

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.