Skip to content

Conversation

@ken00535
Copy link

@ken00535 ken00535 commented Jun 7, 2021

Because of wg's counter can't be a negative number, if call Wait() and then call Add(1) before Wait() complete, it will throw a panic "WaitGroup is reused before previous Wait has returned". so this PR add a mutex to fix the issue.

For more detail, see:
https://stackoverflow.com/questions/48351816/waitgroup-is-reused-before-previous-wait-unknown-reason

I write a test TestSubscribeAsyncWithMultipleGoroutine that can reproduce the issue.

func TestSubscribeAsyncWithMultipleGoroutine(t *testing.T) {
	for i := 0; i < 100; i++ {
		bus := New()
		bus.SubscribeAsync("topic", func() {
			time.Sleep(time.Millisecond)
		}, false)
		bus.Publish("topic")
		go func() {
			time.Sleep(time.Millisecond)
			bus.Publish("topic")
		}()
		bus.WaitAsync()
	}
}

because of wg's counter can't be a negative number, if call Wait() and then call Add(1) before
Wait() complete, it will throw panic "WaitGroup is reused before previous Wait has returned". so add
a mutex to prevent this issue. for more detail, see:
https://stackoverflow.com/questions/48351816/waitgroup-is-reused-before-previous-wait-unknown-reason
@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2021

Codecov Report

Merging #49 (0c59636) into master (49d4230) will decrease coverage by 1.40%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #49      +/-   ##
==========================================
- Coverage   83.26%   81.86%   -1.41%     
==========================================
  Files           4        4              
  Lines         251      215      -36     
==========================================
- Hits          209      176      -33     
+ Misses         29       26       -3     
  Partials       13       13              
Impacted Files Coverage Δ
event_bus.go 93.75% <100.00%> (-1.05%) ⬇️
network_bus.go 76.66% <0.00%> (-2.75%) ⬇️
server.go 72.13% <0.00%> (-1.79%) ⬇️
client.go 77.27% <0.00%> (+0.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 49d4230...0c59636. Read the comment docs.

@HildaM
Copy link

HildaM commented May 21, 2024

There is a bug!
If you add lock on WaitAsync() function, it will cause TestSubcribeOnceAsync() dead lock.

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