Skip to content
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

These ones were the ones testing Open scenarios. The issue is that Op… #57

Merged
merged 2 commits into from
Mar 25, 2022

Conversation

DanielePalaia
Copy link
Contributor

…en and Close, rwc.Open and rwc.Close can at the same time write on:

c.allocator = newAllocator(1, c.Config.ChannelMax)
connection.go line 444 and
connection.go line 849

while shutdown is protected by the structure mutex m, OpenComplete() is not causing the race.

While it's not clear if the library should protect this eventuality, the tests are testing the Open function, so I think the close can be put in the main thread avoiding the race and not affecting the test validity

Daniele Palaia added 2 commits March 25, 2022 15:07
…en and Close, rwc.Open and rwc.Close can at the same time write on:

c.allocator = newAllocator(1, c.Config.ChannelMax)
connection.go line 444 and
connection.go line 849

while shutdown is protected by the structure mutex m, OpenComplete() is not causing the race.

While it's not clear if the library should protect this eventuality, the tests are testing the Open function, so I think the close can be put in the main thread avoiding the race and not affecting the test validity
Copy link
Contributor

@Zerpet Zerpet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@DanielePalaia
Copy link
Contributor Author

Thank you!

Thanks to you! @Zerpet @Gsantomaggio

@DanielePalaia DanielePalaia merged commit 3d56e20 into main Mar 25, 2022
@DanielePalaia DanielePalaia deleted the unit_test_race_condition branch March 25, 2022 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants