fix data race from getDefaultTransport 这个api会产生data race #218
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
感谢您提出Pull Request，我会尽快Review。我会在1-2日内进行查看或者回复，如果遇到节假日可能会处理较慢，敬请谅解。 |
zouyx
reviewed
Mar 23, 2022
protocol/http/request.go
Outdated
| ) | ||
|
|
||
| func getDefaultTransport(insecureSkipVerify bool) *http.Transport { | ||
| lock.Lock() |
There was a problem hiding this comment.
如果想最简单最优雅，可以这样改：
func getDefaultTransport(insecureSkipVerify bool) *http.Transport {
once.Do(func() {
defaultTransport = &http.Transport{
MaxIdleConns: defaultMaxConnsPerHost,
MaxIdleConnsPerHost: defaultMaxConnsPerHost,
DialContext: (&net.Dialer{
KeepAlive: defaultKeepAliveSecond,
Timeout: defaultTimeoutBySecond,
}).DialContext,
}
if insecureSkipVerify {
defaultTransport.TLSClientConfig = &tls.Config{
InsecureSkipVerify: insecureSkipVerify,
}
}
})
return defaultTransport
}
我看defaultTransport这个单例只有在这个函数里有赋值操作，所以不做nil检查也是可以的。如果不放心就按你说的，在once.Do的入参函数里先做nil检查再赋值。
There was a problem hiding this comment.
你告诉我怎么改，我马上改好再提一次
There was a problem hiding this comment.
如果想最简单最优雅，可以这样改：
func getDefaultTransport(insecureSkipVerify bool) *http.Transport { once.Do(func() { defaultTransport = &http.Transport{ MaxIdleConns: defaultMaxConnsPerHost, MaxIdleConnsPerHost: defaultMaxConnsPerHost, DialContext: (&net.Dialer{ KeepAlive: defaultKeepAliveSecond, Timeout: defaultTimeoutBySecond, }).DialContext, } if insecureSkipVerify { defaultTransport.TLSClientConfig = &tls.Config{ InsecureSkipVerify: insecureSkipVerify, } } }) return defaultTransport }我看
defaultTransport这个单例只有在这个函数里有赋值操作，所以不做nil检查也是可以的。如果不放心就按你说的，在once.Do的入参函数里先做nil检查再赋值。
其实我的意思就是这个- -
Pull Request Test Coverage Report for Build 2019812893Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
麻烦再看一下最新的commit：55ac04e |
zouyx
approved these changes
Mar 23, 2022
|
@wubin1989 已发布 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
我把控制台打印日志贴出来供参考：
==================
WARNING: DATA RACE
Read at 0x000006033768 by goroutine 7:
github.com/apolloconfig/agollo/v4/protocol/http.getDefaultTransport()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/protocol/http/request.go:63 +0x31
github.com/apolloconfig/agollo/v4/protocol/http.Request()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/protocol/http/request.go:111 +0x1e4
github.com/apolloconfig/agollo/v4/component/serverlist.SyncServerIPList()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/component/serverlist/sync.go:90 +0x29c
github.com/apolloconfig/agollo/v4/component/serverlist.(*SyncServerIPListComponent).Start()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/component/serverlist/sync.go:56 +0x44
github.com/apolloconfig/agollo/v4/component.StartRefreshConfig()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/component/common.go:27 +0x37
github.com/apolloconfig/agollo/v4/component/serverlist.InitSyncServerIPList·dwrap·1()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/component/serverlist/sync.go:45 +0x47
Previous write at 0x000006033768 by main goroutine:
github.com/apolloconfig/agollo/v4/protocol/http.getDefaultTransport.func1()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/protocol/http/request.go:65 +0x270
sync.(*Once).doSlow()
/usr/local/go/src/sync/once.go:68 +0x127
sync.(*Once).Do()
/usr/local/go/src/sync/once.go:59 +0x46
github.com/apolloconfig/agollo/v4/protocol/http.getDefaultTransport()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/protocol/http/request.go:64 +0x6f
github.com/apolloconfig/agollo/v4/protocol/http.Request()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/protocol/http/request.go:111 +0x1e4
github.com/apolloconfig/agollo/v4/protocol/http.RequestRecovery()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/protocol/http/request.go:203 +0x186
github.com/apolloconfig/agollo/v4/component/remote.(*AbsApolloConfig).SyncWithNamespace()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/component/remote/abs.go:58 +0x4cd
github.com/apolloconfig/agollo/v4/component/remote.(*syncApolloConfig).Sync.func1()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/component/remote/sync.go:100 +0x9c
github.com/apolloconfig/agollo/v4/env/config.SplitNamespaces()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/env/config/config.go:116 +0x1ac
github.com/apolloconfig/agollo/v4/component/remote.(*syncApolloConfig).Sync()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/component/remote/sync.go:99 +0x175
github.com/apolloconfig/agollo/v4.StartWithConfig()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/client.go:124 +0x2c9
github.com/unionj-cloud/go-doudou/framework/configmgr.LoadFromApollo()
/Users/wubin1989/workspace/cloud/go-doudou/framework/configmgr/apollo.go:19 +0x6f
github.com/unionj-cloud/go-doudou/framework/internal/config.init.0()
/Users/wubin1989/workspace/cloud/go-doudou/framework/internal/config/config.go:67 +0x719
Goroutine 7 (running) created at:
github.com/apolloconfig/agollo/v4/component/serverlist.InitSyncServerIPList()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/component/serverlist/sync.go:45 +0xcb
github.com/apolloconfig/agollo/v4.StartWithConfig()
/Users/wubin1989/go/pkg/mod/github.com/apolloconfig/agollo/v4@v4.1.0/client.go:121 +0x248
github.com/unionj-cloud/go-doudou/framework/configmgr.LoadFromApollo()
/Users/wubin1989/workspace/cloud/go-doudou/framework/configmgr/apollo.go:19 +0x6f
github.com/unionj-cloud/go-doudou/framework/internal/config.init.0()
/Users/wubin1989/workspace/cloud/go-doudou/framework/internal/config/config.go:67 +0x719