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.

Sign up for GitHub

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

Jump to bottom

Clean-up and remove remaining data races #5

Merged
merged 7 commits into from
Jan 4, 2023
Merged

Clean-up and remove remaining data races #5

merged 7 commits into from
Jan 4, 2023

Conversation

xiam
Copy link
Contributor

@xiam xiam commented Jan 4, 2023 •
edited
Loading

There were some rare data races remaining like

==================
WARNING: DATA RACE
Write at 0x00c000178038 by goroutine 9:
  github.com/goware/pubsub/redisbus.(*RedisBus[...]).connectAndConsume.func1()
      /home/vagrant/projects/goware/pubsub/redisbus/redisbus.go:275 +0xbc
  runtime.deferreturn()
      /usr/local/go/src/runtime/panic.go:476 +0x32
  github.com/goware/pubsub/redisbus.(*RedisBus[...]).Run()
      /home/vagrant/projects/goware/pubsub/redisbus/redisbus.go:116 +0x6be
  github.com/goware/pubsub/redisbus.TestRedisbusSendLargeAmount.func1()
      /home/vagrant/projects/goware/pubsub/redisbus/redisbus_test.go:267 +0x5a

Previous read at 0x00c000178038 by goroutine 11:
  github.com/goware/pubsub/redisbus.(*RedisBus[...]).consumeMessages.func1()
      /home/vagrant/projects/goware/pubsub/redisbus/redisbus.go:354 +0x109

Goroutine 9 (running) created at:
  github.com/goware/pubsub/redisbus.TestRedisbusSendLargeAmount()
      /home/vagrant/projects/goware/pubsub/redisbus/redisbus_test.go:266 +0x471
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      /usr/local/go/src/testing/testing.go:1493 +0x47

This PR will remove those data races and clean up the package

@xiam xiam requested a review from pkieltyka January 4, 2023 17:35
xiam
xiam commented Jan 4, 2023
View reviewed changes
redisbus/redisbus.go
@@ -235,91 +224,43 @@ func (r *RedisBus[M]) NumSubscribers(channelID string) (int, error) {
return 0, fmt.Errorf("redisbus: pubsub is not running")
}

vs, err := r.client.PubSubNumSub(r.ctx, r.namespace+channelID).Result()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pkieltyka I completed this method and added some code to test it.

xiam
xiam commented Jan 4, 2023
View reviewed changes
redisbus/redisbus.go
if err != nil {
// TODO: timeout review..?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're using Receive() which is blocking and won't return a timeout, we can remove this bit.

xiam
xiam commented Jan 4, 2023
View reviewed changes
_examples/demo-redisbus-chat/main.go
@@ -9,7 +9,7 @@ import (
"sync"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated demos.

@xiam xiam merged commit 26c818b into goware-channel-go-redis Jan 4, 2023
@xiam xiam deleted the goware-channel-go-redis-xiam branch January 4, 2023 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkieltyka pkieltyka pkieltyka approved these changes

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
@xiam @pkieltyka