-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix data-race warnings #4036
fix data-race warnings #4036
Conversation
upstream pr: projectdiscovery/utils#235 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm ! upstream PR might need some changes as suggested by @Mzack9999
@@ -71,6 +72,7 @@ func (e *Executer) Execute(input *contextargs.Context) (bool, error) { | |||
} | |||
previous := make(map[string]interface{}) | |||
|
|||
mtx := &sync.Mutex{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this mutex seems unnecessary as the variable has write-only operations within two mutually exclusive code branches. Could you verify if removing it reintroduces somehow the data race?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes @Mzack9999, I have tried again somehow it's reintroduces data race issue without a mutex wrapper.
[openssh-detect] [tcp] [info] scanme.sh:22 [SSH-2.0-OpenSSH_8.2p1 Ubuntu-4ubuntu0.9]
==================
WARNING: DATA RACE
Write at 0x00c00d251da0 by goroutine 7295350:
github.com/projectdiscovery/nuclei/v2/pkg/protocols/common/executer.(*Executer).Execute.func3()
/Users/ramana/projectdiscovery-workspace/nuclei/v2/pkg/protocols/common/executer/executer.go:
Proposed changes
WARNING: DATA RACE #4016
Data race at: github.com/projectdiscovery/utils/conn/connpool.(*OneTimePool).Close()
test
✗ go run -race . -l ../functional-test/targets.txt
Note:
../functional-test/targets.txt
contains 150 targetsChecklist