Skip to content

Fix DATA RACE as a result of changing ber's module global variable for fuzz tests#473

Merged
johnweldon merged 1 commit intogo-ldap:masterfrom
t2y:fix-fuzz-test
Nov 21, 2023
Merged

Fix DATA RACE as a result of changing ber's module global variable for fuzz tests#473
johnweldon merged 1 commit intogo-ldap:masterfrom
t2y:fix-fuzz-test

Conversation

@t2y
Copy link
Contributor

@t2y t2y commented Nov 20, 2023

I investigated why DATA RACE occurred.

WARNING: DATA RACE
Write at 0x000000c3c628 by goroutine 38:
  github.com/go-ldap/ldap.FuzzParseDN()
      /home/runner/work/ldap/ldap/fuzz_test.go:14 +0x32
  testing.fRunner()
      /opt/hostedtoolcache/go/1.18.10/x64/src/testing/fuzz.go:700 +0x168
  testing.runFuzzTests.func1()
      /opt/hostedtoolcache/go/1.18.10/x64/src/testing/fuzz.go:5[20](https://github.com/go-ldap/ldap/actions/runs/6781841980/job/18432888679#step:5:21) +0x47

Fuzz tests also run as unit tests, so the module global variable shouldn't change in a test function.

ber.MaxPacketLengthBytes = 65536

I fix this conflict by adding TestMain(), which handles a setup process before all tests. Now, all tests run with ber.MaxPacketLengthBytes limitation. Does it make sense?

See also #472.

References

@t2y
fix: add a setup process in TestMain to prevent DATA RACE as a result…
b47d9e3 
… of changing ber's module global variable for fuzz tests go-ldap#472
@t2y t2y mentioned this pull request Nov 20, 2023
Merged
@johnweldon
Copy link
Member

johnweldon commented Nov 21, 2023

Thank you @t2y
We do want this in the v3 folder eventually, but for now this will get you past the CI errors.

@johnweldon johnweldon merged commit ef0e538 into go-ldap:master Nov 21, 2023
@t2y t2y deleted the fix-fuzz-test branch November 21, 2023 00:07
gustavoluvizotto pushed a commit to gustavoluvizotto/ldap-fork that referenced this pull request Apr 11, 2024
@t2y @gustavoluvizotto
fix: add a setup process in TestMain to prevent DATA RACE as a result…
3cacdbe 
… of changing ber's module global variable for fuzz tests go-ldap#472 (go-ldap#473)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@t2y @johnweldon