jsonrpc2: initialize Connection.async before calling binder.Bind #45
Conversation
maxatome force-pushed the fix-conn-bind branch from a29a07e to 0ef160d Compare 
| This PR (HEAD: 0ef160d) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/exp/+/436888 to see it. Tip: You can toggle comments from me using the |
| Fixes golang/go/issues/56131 |
| Message from Bryan Mills: Patch Set 1: (3 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/436888. |
| The "Fixes" comment needs to go into the initial comment describing this pull request, so that it gets copied into Gerritt. See https://go.dev/wiki/Gerritbot. Thanks. |
| This PR (HEAD: 477196f) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/exp/+/436888 to see it. Tip: You can toggle comments from me using the |
| Message from Gopher Robot: Patch Set 2: Congratulations on opening your first change. Thank you for your contribution! Next steps: Most changes in the Go project go through a few rounds of revision. This can be Please don’t reply on this GitHub thread. Visit golang.org/cl/436888. |
maxatome force-pushed the fix-conn-bind branch from 477196f to 9067b31 Compare | This PR (HEAD: 9067b31) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/exp/+/436888 to see it. Tip: You can toggle comments from me using the |
If Connection.async is not initialized before calling binder.Bind() and a goroutine is spawned in binder.Bind() to monitor the end of the connection using conn.Wait(), this conn.Wait() can wait forever: func (s *MyServer) Bind( ctx context.Context, conn *jsonrpc2.Connection, ) (jsonrpc2.ConnectionOptions, error) { // XXX register conn in *MyServer // monitor the end of the connection go func() { conn.Wait() conn.Close() // YYY do some cleanup in *MyServer }() return jsonrpc2.ConnectionOptions{}, nil } The "// YYY do some cleanup in *MyServer" should be reached for each end of connection. If Connection.async is initialized too late, after conn.Wait() is called in the Bind() spawned goroutine, conn.Wait() never returns as it reads on a nil conn.async.ready channel. Signed-off-by: Maxime Soulé <btik-git@scoubidou.com> maxatome force-pushed the fix-conn-bind branch from 9067b31 to a7fce45 Compare | This PR (HEAD: a7fce45) has been imported to Gerrit for code review. Please visit https://go-review.googlesource.com/c/exp/+/436888 to see it. Tip: You can toggle comments from me using the |
| Message from Maxime Soulé: Patch Set 4: (4 comments) Please don’t reply on this GitHub thread. Visit golang.org/cl/436888. |
| Message from Bryan Mills: Patch Set 4: Run-TryBot+1 Auto-Submit+1 Code-Review+2 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/436888. |
| Message from Bryan Mills: Patch Set 4: -Auto-Submit Please don’t reply on this GitHub thread. Visit golang.org/cl/436888. |
| Message from Gopher Robot: Patch Set 4: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/436888. |
| Message from Gopher Robot: Patch Set 4: TryBot-Result+1 (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/436888. |
| Message from Maxime Soulé: Patch Set 5: (1 comment) Please don’t reply on this GitHub thread. Visit golang.org/cl/436888. |
| Message from Bryan Mills: Patch Set 5: Run-TryBot+1 Auto-Submit+1 Code-Review+2 Please don’t reply on this GitHub thread. Visit golang.org/cl/436888. |
If Connection.async is not initialized before calling binder.Bind() and a goroutine is spawned in binder.Bind() to monitor the end of the connection using conn.Wait(), this conn.Wait() can wait forever: func (s *MyServer) Bind( ctx context.Context, conn *jsonrpc2.Connection, ) (jsonrpc2.ConnectionOptions, error) { // XXX register conn in *MyServer // monitor the end of the connection go func() { conn.Wait() conn.Close() // YYY do some cleanup in *MyServer }() return jsonrpc2.ConnectionOptions{}, nil } The "// YYY do some cleanup in *MyServer" should be reached for each end of connection. If Connection.async is initialized too late, after conn.Wait() is called in the Bind() spawned goroutine, conn.Wait() never returns as it reads on a nil conn.async.ready channel. Fixes #56131. Change-Id: I3c6e69d7dcc860479c4518ebd18e2431606322bc GitHub-Last-Rev: a7fce45 GitHub-Pull-Request: #45 Reviewed-on: https://go-review.googlesource.com/c/exp/+/436888 Run-TryBot: Bryan Mills <bcmills@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Bryan Mills <bcmills@google.com> Reviewed-by: Joedian Reid <joedian@golang.org> | This PR is being closed because golang.org/cl/436888 has been merged. |
3 participants
If Connection.async is not initialized before calling binder.Bind()
and a goroutine is spawned in binder.Bind() to monitor the end of the
connection using conn.Wait(), this conn.Wait() can wait forever:
The "// YYY do some cleanup in *MyServer" should be reached for each
end of connection.
If Connection.async is initialized too late, after conn.Wait() is
called in the Bind() spawned goroutine, conn.Wait() never returns as
it reads on a nil conn.async.ready channel.
Fixes #56131.