Repository: cloudwego/hertz
Pull request #149 (2022-07-22 09:05:46)

fix: client test race
#### What type of PR is this?
fix

#### Check the PR title.
<!--
The description of the title will be attached in Release Notes, 
so please describe it from user-oriented, what this PR does / why we need it.
Please check your PR title with the below requirements:
-->
- [x] This PR title match the format: \<type\>(optional scope): \<description\>
- [x] The description of this PR title is user-oriented and clear enough for others to understand.


#### (Optional) Translate the PR title into Chinese.
修复 hostclient race 可能导致的串连接问题

#### (Optional) More detail description for this PR(en: English/zh: Chinese).
en: Fix the wrong connection problem that may be caused by hostclient race
zh: 修复 hostclient race 可能导致的串连接问题

#### Which issue(s) this PR fixes:
<!--
Automatically closes linked issue when PR is merged.
Eg: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->


codecov[bot]: # [Codecov](https://codecov.io/gh/cloudwego/hertz/pull/149?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) Report
> Merging [#149](https://codecov.io/gh/cloudwego/hertz/pull/149?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) (6c65289) into [develop](https://codecov.io/gh/cloudwego/hertz/commit/dcb0b5a1861a1e2e4629a8607a3ca9c89009ac55?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) (dcb0b5a) will **decrease** coverage by `0.04%`.
> The diff coverage is `n/a`.

```diff
@@             Coverage Diff             @@
##           develop     #149      +/-   ##
===========================================
- Coverage    59.14%   59.09%   -0.05%     
===========================================
  Files           79       79              
  Lines         8111     8111              
===========================================
- Hits          4797     4793       -4     
- Misses        2960     2964       +4     
  Partials       354      354              
```


| [Impacted Files](https://codecov.io/gh/cloudwego/hertz/pull/149?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) | Coverage Δ | |
|---|---|---|
| [pkg/protocol/http1/client.go](https://codecov.io/gh/cloudwego/hertz/pull/149/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego#diff-cGtnL3Byb3RvY29sL2h0dHAxL2NsaWVudC5nbw==) | `35.54% <ø> (ø)` | |
| [pkg/common/timer/timer.go](https://codecov.io/gh/cloudwego/hertz/pull/149/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego#diff-cGtnL2NvbW1vbi90aW1lci90aW1lci5nbw==) | `61.90% <0.00%> (-19.05%)` | :arrow_down: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego)


Patch:
diff --git a/pkg/protocol/client/client.go b/pkg/protocol/client/client.go
index 962abfb..e5e6cf1 100644
--- a/pkg/protocol/client/client.go
+++ b/pkg/protocol/client/client.go
@@ -43,7 +43,6 @@ package client
 
 import (
 	"context"
-	"crypto/tls"
 	"sync"
 	"time"
 
@@ -79,34 +78,6 @@ type Doer interface {
 	Do(ctx context.Context, req *protocol.Request, resp *protocol.Response) error
 }
 
-type HostClientConfig struct {
-	DynamicConfig
-
-	Name string
-
-	NoDefaultUserAgentHeader      bool
-	DialDualStack                 bool
-	DisableHeaderNamesNormalizing bool
-	DisablePathNormalizing        bool
-	IsTLS                         bool
-
-	TLSConfig *tls.Config
-
-	MaxConns                  int
-	MaxIdempotentCallAttempts int
-	MaxResponseBodySize       int
-
-	RetryIf            RetryIfFunc
-	ResponseBodyStream bool
-
-	DialTimeout         time.Duration
-	MaxIdleConnDuration time.Duration
-	MaxConnDuration     time.Duration
-	ReadTimeout         time.Duration
-	WriteTimeout        time.Duration
-	MaxConnWaitTimeout  time.Duration
-}
-
 // DynamicConfig is config set which will be confirmed when starts a request.
 type DynamicConfig struct {
 	Addr     string
diff --git a/pkg/protocol/http1/client.go b/pkg/protocol/http1/client.go
index 12d2ccc..4be7be1 100644
--- a/pkg/protocol/http1/client.go
+++ b/pkg/protocol/http1/client.go
@@ -90,6 +90,19 @@ type HostClient struct {
 
 	*ClientOptions
 
+	// Comma-separated list of upstream HTTP server host addresses,
+	// which are passed to Dialer in a round-robin manner.
+	//
+	// Each address may contain port if default dialer is used.
+	// For example,
+	//
+	//    - foobar.com:80
+	//    - foobar.com:443
+	//    - foobar.com:8080
+	Addr     string
+	IsTLS    bool
+	ProxyURI *protocol.URI
+
 	clientName  atomic.Value
 	lastUseTime uint32
 
@@ -1044,17 +1057,6 @@ func NewHostClient(c *ClientOptions) client.HostClient {
 }
 
 type ClientOptions struct {
-	// Comma-separated list of upstream HTTP server host addresses,
-	// which are passed to Dialer in a round-robin manner.
-	//
-	// Each address may contain port if default dialer is used.
-	// For example,
-	//
-	//    - foobar.com:80
-	//    - foobar.com:443
-	//    - foobar.com:8080
-	Addr string
-
 	// Client name. Used in User-Agent request header.
 	Name string
 
@@ -1086,8 +1088,6 @@ type ClientOptions struct {
 	// Optional TLS config.
 	TLSConfig *tls.Config
 
-	IsTLS bool
-
 	// Maximum number of connections which may be established to all hosts
 	// listed in Addr.
 	//
@@ -1168,6 +1168,4 @@ type ClientOptions struct {
 
 	// ResponseBodyStream enables response body streaming
 	ResponseBodyStream bool
-
-	ProxyURI *protocol.URI
 }


Test patch:
diff --git a/pkg/app/client/client_test.go b/pkg/app/client/client_test.go
index 0efe406..20c25e7 100644
--- a/pkg/app/client/client_test.go
+++ b/pkg/app/client/client_test.go
@@ -392,8 +392,8 @@ func TestClientReadTimeout(t *testing.T) {
 			ReadTimeout:               time.Second * 4,
 			MaxIdempotentCallAttempts: 1,
 			Dialer:                    standard.NewDialer(),
-			Addr:                      opt.Addr,
 		},
+		Addr: opt.Addr,
 	}
 
 	req := protocol.AcquireRequest()
@@ -689,9 +689,9 @@ func TestHostClientPendingRequests(t *testing.T) {
 
 	c := &http1.HostClient{
 		ClientOptions: &http1.ClientOptions{
-			Addr:   "foobar",
 			Dialer: newMockDialerWithCustomFunc(opt.Network, opt.Addr, time.Second, nil),
 		},
+		Addr: "foobar",
 	}
 
 	pendingRequests := c.PendingRequests()
@@ -780,10 +780,10 @@ func TestHostClientMaxConnsWithDeadline(t *testing.T) {
 
 	c := &http1.HostClient{
 		ClientOptions: &http1.ClientOptions{
-			Addr:     "foobar",
 			Dialer:   newMockDialerWithCustomFunc(opt.Network, opt.Addr, time.Second, nil),
 			MaxConns: 1,
 		},
+		Addr: "foobar",
 	}
 
 	for i := 0; i < 5; i++ {
@@ -849,10 +849,10 @@ func TestHostClientMaxConnDuration(t *testing.T) {
 
 	c := &http1.HostClient{
 		ClientOptions: &http1.ClientOptions{
-			Addr:            "foobar",
 			Dialer:          newMockDialerWithCustomFunc(opt.Network, opt.Addr, time.Second, nil),
 			MaxConnDuration: 10 * time.Millisecond,
 		},
+		Addr: "foobar",
 	}
 
 	for i := 0; i < 5; i++ {
@@ -895,11 +895,11 @@ func TestHostClientMultipleAddrs(t *testing.T) {
 	dialsCount := make(map[string]int)
 	c := &http1.HostClient{
 		ClientOptions: &http1.ClientOptions{
-			Addr: "foo,bar,baz",
 			Dialer: newMockDialerWithCustomFunc(opt.Network, opt.Addr, 1*time.Second, func(network, addr string, timeout time.Duration, tlsConfig *tls.Config) {
 				dialsCount[addr]++
 			}),
 		},
+		Addr: "foo,bar,baz",
 	}
 
 	for i := 0; i < 9; i++ {
@@ -961,9 +961,9 @@ func TestClientFollowRedirects(t *testing.T) {
 
 	c := &http1.HostClient{
 		ClientOptions: &http1.ClientOptions{
-			Addr:   "xxx",
 			Dialer: newMockDialerWithCustomFunc(opt.Network, opt.Addr, 1*time.Second, nil),
 		},
+		Addr: "xxx",
 	}
 
 	for i := 0; i < 10; i++ {
@@ -1055,11 +1055,11 @@ func TestHostClientMaxConnWaitTimeoutSuccess(t *testing.T) {
 
 	c := &http1.HostClient{
 		ClientOptions: &http1.ClientOptions{
-			Addr:               "foobar",
 			Dialer:             newMockDialerWithCustomFunc(opt.Network, opt.Addr, time.Second, nil),
 			MaxConns:           1,
 			MaxConnWaitTimeout: 200 * time.Millisecond,
 		},
+		Addr: "foobar",
 	}
 
 	for i := 0; i < 5; i++ {
@@ -1124,11 +1124,11 @@ func TestHostClientMaxConnWaitTimeoutError(t *testing.T) {
 
 	c := &http1.HostClient{
 		ClientOptions: &http1.ClientOptions{
-			Addr:               "foobar",
 			Dialer:             newMockDialerWithCustomFunc(opt.Network, opt.Addr, time.Second, nil),
 			MaxConns:           1,
 			MaxConnWaitTimeout: 10 * time.Millisecond,
 		},
+		Addr: "foobar",
 	}
 
 	var errNoFreeConnsCount uint32
diff --git a/pkg/protocol/http1/client_test.go b/pkg/protocol/http1/client_test.go
index ae0871a..299a028 100644
--- a/pkg/protocol/http1/client_test.go
+++ b/pkg/protocol/http1/client_test.go
@@ -74,13 +74,13 @@ func TestHostClientMaxConnWaitTimeoutWithEarlierDeadline(t *testing.T) {
 
 	c := &HostClient{
 		ClientOptions: &ClientOptions{
-			Addr: "foobar",
 			Dialer: newSlowConnDialer(func(network, addr string) (network.Conn, error) {
 				return mock.SlowReadDialer(addr)
 			}),
 			MaxConns:           1,
 			MaxConnWaitTimeout: 50 * time.Millisecond,
 		},
+		Addr: "foobar",
 	}
 
 	var errTimeoutCount uint32