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

Race condition when running the redis example #364

Closed
donstephan opened this issue Jan 24, 2024 · 4 comments
Closed

Race condition when running the redis example #364

donstephan opened this issue Jan 24, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@donstephan
Copy link

donstephan commented Jan 24, 2024

When running the redis example with the golang -race flag it outputs a data race when it initially starts up. I haven't observed this happening in a live environment but noticed it when I was doing some other debugging.

WARNING: DATA RACE
Write at 0x00c0003280a0 by goroutine 15:
  sync/atomic.StoreInt64()
      /usr/local/go/src/runtime/race_amd64.s:237 +0xb
  sync/atomic.StoreInt64()
      <autogenerated>:1 +0x15
  github.com/mochi-mqtt/server/v2.(*Server).eventLoop()
      /server/server.go:309 +0x41c
  github.com/mochi-mqtt/server/v2.(*Server).Serve.func2()
      /server/server.go:290 +0x33

Previous read at 0x00c0003280a0 by goroutine 14:
  github.com/mochi-mqtt/server/v2/hooks/storage/redis.(*Hook).OnSysInfoTick()
      /server/hooks/storage/redis/redis.go:364 +0x124
  github.com/mochi-mqtt/server/v2.(*Hooks).OnSysInfoTick()
      /server/hooks.go:197 +0x114
  github.com/mochi-mqtt/server/v2.(*Server).publishSysTopics()
      /server/server.go:1384 +0x16f6
  github.com/mochi-mqtt/server/v2.(*Server).Serve()
      /server/server.go:292 +0x2d7
  main.main.func2()
      /server/examples/persistence/redis/main.go:58 +0x2e

Goroutine 15 (running) created at:
  github.com/mochi-mqtt/server/v2.(*Server).Serve()
      /server/server.go:290 +0x22a
  main.main.func2()
      /server/examples/persistence/redis/main.go:58 +0x2e

Goroutine 14 (finished) created at:
  main.main()
      /server/examples/persistence/redis/main.go:57 +0xa04
==================

Clone the repo, start a redis server on localhost:6379 and run:

go run -race ./examples/persistence/redis/main.go

I will provide a fix when I get the time to do so.

@donstephan
Copy link
Author

donstephan commented Jan 24, 2024

Issue is here:

go s.eventLoop() // spin up event loop for issuing $SYS values and closing server.

go s.eventLoop() starts and the the s.publishSysTopics() runs right after it which could race with channels in the event loop. Would be very rare (if not impossible) to hit. If I'm reading it right, you'd somehow need a pause of 1 second between those two function executions to encounter this condition. Easiest fix is to just move the s.publishSysTopics() into the eventLoop() function so it runs before it listens to the tickers. Or it could just be moved above the eventLoop() execution in the server file.

@thedevop thedevop added the bug Something isn't working label Jan 25, 2024
@thedevop
Copy link
Collaborator

thedevop commented Jan 25, 2024

Thanks @donstephan for reporting this!

The issue is *system.Info was passed into hooks, and in the Redis hook, it directly dereferenced the live info:

.

I have provided a fix in PR #365, please verify.

@donstephan
Copy link
Author

Looks great. Tested the PR with the redis example and no more race. Thank you much!

@thedevop
Copy link
Collaborator

thedevop commented Feb 6, 2024

@donstephan , this is incorporated into v2.4.6.

@thedevop thedevop closed this as completed Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants