services/horizon: Fix data races found by integration tests (#3690)

This commit adds `-race` flag to Horizon integration tests and fixes two bugs found by race detector: * `Client.sendRequest` overwriting `c.HorizonURL` when used in multiple go routines. * `decoder.IgnoreUnknownKeys` overwriting `gorilla/schema.Decoder.ignoreUnknownKeys` when called from multiple go routines.
stellar · Jun 15, 2021 · 55f106c · 55f106c
1 parent 226d70a
commit 55f106c
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 6 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -448,7 +448,7 @@ jobs:
  # Currently all integration tests are in a single directory.
  command: |
  cd ~/go/src/github.com/stellar/go
- go test -timeout 25m -v ./services/horizon/internal/integration/...
+ go test -race -timeout 25m -v ./services/horizon/internal/integration/...
 
 #-------------------------------------------------------------------------#
 # Workflows orchestrate jobs and make sure they run in the right sequence #

diff --git a/clients/horizonclient/CHANGELOG.md b/clients/horizonclient/CHANGELOG.md
@@ -7,6 +7,7 @@ file. This project adheres to [Semantic Versioning](http://semver.org/).
 
 * Added transaction and operation result codes to the horizonclient.Error string for easy glancing at string only errors for underlying cause.
 * Fix bug in the transaction submission where requests with large transaction payloads fail with an HTTP 414 URI Too Long error ([#3643](https://github.com/stellar/go/pull/3643)).
+* Fix a data race in `Client.fixHorizonURL`([#3690](https://github.com/stellar/go/pull/3690)).
 
 ## [v7.0.0](https://github.com/stellar/go/releases/tag/horizonclient-v7.0.0) - 2021-05-15
 

diff --git a/clients/horizonclient/client.go b/clients/horizonclient/client.go
@@ -26,8 +26,7 @@ import (
 
 // sendRequest builds the URL for the given horizon request and sends the url to a horizon server
 func (c *Client) sendRequest(hr HorizonRequest, resp interface{}) (err error) {
- c.HorizonURL = c.fixHorizonURL()
- req, err := hr.HTTPRequest(c.HorizonURL)
+ req, err := hr.HTTPRequest(c.fixHorizonURL())
  if err != nil {
  return err
  }
@@ -270,7 +269,10 @@ func (c *Client) setDefaultClient() {
 
 // fixHorizonURL strips all slashes(/) at the end of HorizonURL if any, then adds a single slash
 func (c *Client) fixHorizonURL() string {
- return strings.TrimRight(c.HorizonURL, "/") + "/"
+ c.fixHorizonURLOnce.Do(func() {
+ c.HorizonURL = strings.TrimRight(c.HorizonURL, "/") + "/"
+ })
+ return c.HorizonURL
 }
 
 // SetHorizonTimeout allows users to set the timeout before a horizon request is cancelled.

diff --git a/clients/horizonclient/main.go b/clients/horizonclient/main.go
@@ -129,7 +129,8 @@ type UniversalTimeHandler func() int64
 // Client struct contains data for creating a horizon client that connects to the stellar network.
 type Client struct {
  // URL of Horizon server to connect
- HorizonURL string
+ HorizonURL string
+ fixHorizonURLOnce sync.Once
 
  // HTTP client to make requests with
  HTTP HTTP

diff --git a/services/horizon/internal/actions/helpers.go b/services/horizon/internal/actions/helpers.go
@@ -399,7 +399,6 @@ func getParams(dst interface{}, r *http.Request) error {
  }
  }
 
- decoder.IgnoreUnknownKeys(true)
  if err := decoder.Decode(dst, query); err != nil {
  for k, e := range err.(schema.MultiError) {
  return problem.NewProblemWithInvalidField(
@@ -609,3 +608,7 @@ func countNonEmpty(params ...interface{}) (int, error) {
 
  return count, nil
 }
+
+func init() {
+ decoder.IgnoreUnknownKeys(true)
+}