search

StalkR / goircbot

Go IRC Bot
https://godoc.org/github.com/StalkR/goircbot
Apache License 2.0
36 stars 8 forks source link

Data race in df plugin, or any plugin that sends from a goroutine #8

Closed mpe closed 9 years ago

mpe commented 9 years ago

Using the example bot, with the df limit altered so it fires, I see:

$ go run -race examplebot.go -host localhost:6667 -channels "#test"
...
WARNING: DATA RACE
Write by goroutine 77:
  runtime.mapassign1()
      /home/michael/work/topics/golang/go/src/runtime/hashmap.go:374 +0x0
  github.com/fluffle/goirc/state.(*Nick).addChannel()
      ~/golang/src/github.com/fluffle/goirc/state/nick.go:73 +0x151
  github.com/fluffle/goirc/state.(*stateTracker).Associate()
      ~/golang/src/github.com/fluffle/goirc/state/tracker.go:214 +0x3ad
  github.com/fluffle/goirc/client.(*Conn).h_JOIN()
      ~/golang/src/github.com/fluffle/goirc/client/state_handlers.go:77 +0x665
  github.com/fluffle/goirc/client.HandlerFunc.Handle()
      ~/golang/src/github.com/fluffle/goirc/client/dispatch.go:24 +0x43
  github.com/fluffle/goirc/client.(*hNode).Handle()
      ~/golang/src/github.com/fluffle/goirc/client/dispatch.go:52 +0xc8
  github.com/fluffle/goirc/client.func·002()
      ~/golang/src/github.com/fluffle/goirc/client/dispatch.go:128 +0x1c4

Previous read by goroutine 6:
  github.com/fluffle/goirc/state.(*Nick).Channels()
      ~/golang/src/github.com/fluffle/goirc/state/nick.go:124 +0x81
  github.com/StalkR/goircbot/plugins/df.(*Alarm).Notify()
      ~/golang/src/github.com/StalkR/goircbot/plugins/df/handlers.go:62 +0x315
  github.com/StalkR/goircbot/plugins/df.(*Alarm).Monitor()
      ~/golang/src/github.com/StalkR/goircbot/plugins/df/handlers.go:46 +0x2c5

Goroutine 77 (running) created at:
  github.com/fluffle/goirc/client.(*hSet).dispatch()
      ~/golang/src/github.com/fluffle/goirc/client/dispatch.go:130 +0x32c
  github.com/fluffle/goirc/client.(*Conn).dispatch()
      ~/golang/src/github.com/fluffle/goirc/client/dispatch.go:161 +0x5a
  github.com/fluffle/goirc/client.(*Conn).runLoop()
      ~/golang/src/github.com/fluffle/goirc/client/connection.go:316 +0x131

Goroutine 6 (running) created at:
  github.com/StalkR/goircbot/plugins/df.Register()
      ~/golang/src/github.com/StalkR/goircbot/plugins/df/handlers.go:97 +0x15c
  main.main()
      ~/golang/src/github.com/StalkR/goircbot/examplebot.go:63 +0x5c4

This seems to be a basic problem with anything that sends from another goroutine.

I guess the best fix is in goirc, so close this if you like, but I thought I'd at least document it for others.

StalkR commented 9 years ago

Hi Michael, good catch, thanks for the report. Indeed I think it'd be best fixed in goirc by making Nick safe for concurrent access, maybe with a mutex to protect https://github.com/fluffle/goirc/blob/master/state%2Fnick.go#L15. I opened an issue in goirc https://github.com/fluffle/goirc/issues/49

StalkR commented 9 years ago

goirc was refactored to fix this, I updated goircbot accordingly in https://github.com/StalkR/goircbot/commit/d1b83c5e95067ef1af37d1992428b77f84310c25 The df race should be fixed now, do you confirm? please reopen if not. I see another with the renick plugin though, opened https://github.com/StalkR/goircbot/issues/10 to track.

mpe commented 9 years ago

Looks good to me.