Mock.On no longer acquires a lock #1210
Comments
This was referenced Jun 22, 2022
sfc-gh-eraigosa
pushed a commit
to sfc-gh-eraigosa/testify
that referenced
this issue
Jun 23, 2022
This reverts a change that was made in stretchr#1182 The PR makes m.mutex a pointer which now needs to be checked but it's not checked for nil everywhere. This should also help with these issues: - stretchr#1208 - stretchr#1210
boyan-soubachov
pushed a commit
that referenced
this issue
Jun 24, 2022
* fixing panic in calls to assertion with nil m.mutex This reverts a change that was made in #1182 The PR makes m.mutex a pointer which now needs to be checked but it's not checked for nil everywhere. This should also help with these issues: - #1208 - #1210 * Revert throwing out the lock because other concurrent calls can already have it locked * fix go vet copy lock by using pointer * fix obj assignment for passing test
|
This appears to have been addressed by #1212, thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As part of the change in #1182 to change the Mock mutex to a pointer, the
Mock.Onmethod was changed to re-construct the mutex each time it is called.This appears to have also introduced another regression (related to but in addition to #1208): Previously it was safe to call
Onconcurrently with other operations on a Mock, but now becauseOnresets that pointer, concurrent calls can race (observed in v1.7.4).A small repro is attached to tickle the basic issue (
go test -raceshould suffice).Arguably it's silly for us to be calling
Onconcurrently with other operations, but regrettably we have some code that relies on the ability to do so, and it worked before v1.7.4 but no longer does.repro.zip
The text was updated successfully, but these errors were encountered: