joao-r-reis commented on code in PR #1822:
URL:
https://github.com/apache/cassandra-gocql-driver/pull/1822#discussion_r1803383960
##########
conn.go:
##########
@@ -474,8 +483,12 @@ func (s *startupCoordinator) startup(ctx context.Context,
supported map[string][
case error:
return v
case *readyFrame:
+ // Connection is successfully set up and ready to use Native
Protocol v5
+ s.conn.startupCompleted = true
Review Comment:
Ok I understand, the `startupErr` and `frameTicker` channels ensure that
everything is properly synchronized before we return the `conn` object so
there's no race.
I still think this is very obscure as it stands, having shared state like
this is not great... Is there any other field that is being "implictly"
synchronized like this? `frameTicker` wasn't really meant to synchronize shared
state if I understand correctly, it was probably just meant to notify `recv()`
that a response is expected.
I can easily see a bug being introduced in the future because the visibility
of `startupCompleted` is being ensured by channel operations that have a
different main purpose and I doubt a dev looking at this code for the first
time will recognize this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
