Fixed locking between connection_reader and connection#84
Conversation
wolfstudy
left a comment
wolfstudy
left a comment
There was a problem hiding this comment.
The changes LGTM+1.
In here, i am a bit confused. Go officially provided bytes.Buffer can not meet our needs? Why do we have to repackage a Buffer?
|
The package of this Buffer structure is more similar to the way Java buffer is used, but may not be very friendly to developers of the go community. |
| writeRequestsCh chan []byte | ||
|
|
||
| mapMutex sync.RWMutex | ||
| pendingReqs map[uint64]*request |
There was a problem hiding this comment.
Maybe add a comment on how this should only be accessed in certain places?
There was a problem hiding this comment.
Agree with @cckellogg
The pendingReqs was really meant to be accessed by a single go-routine.
pendingReqs not concurrently safe, so it is necessary for the user a comment.
We need more functionalities like efficiently reading int32 and int16 while encapsulating the details.
The ideas of Buffer are taken from Netty ByteBuf (rather than Java ByteBuffer) because the API is an excellent example of good abstractions for working with buffers and parsing/writing network protocols. The abstractions are not language specific. |
|
run integration tests |
3 participants
Motivation
Fixed locking around the
mapMutexin consumer. The current behavior is mixing read-write and read-only locks even when modifying the map.The
pendingReqswas really meant to be accessed by a single go-routine. Changed the logic to pass the received commands on a channel, to be processed in the connection's own go-routine.cc/ @cckellogg