asaskevich / EventBus

[Go] Lightweight eventbus with async compatibility for Go
MIT License
1.74k stars 220 forks source link

Potential deadlock due to calling callbacks while holding a lock #35

Open sergey-shambir opened 5 years ago

sergey-shambir commented 5 years ago

See golang mutexes aren't recursive / re-entrant

Bus.Publish(...) method locks mutex and calls callbacks under lock, so any access to the bus in callback will cause deadlock.

The following example reproduces issue:

package main

import (
    "fmt"

    "github.com/asaskevich/EventBus"
)

var bus EventBus.Bus

func showbug(a int, b int) {
    fmt.Printf("%d\n", a+b)

    if a == 20 {
        bus.Publish("main:calculator", a+1, b)
    }
}

func main() {
    bus = EventBus.New()
    bus.Subscribe("main:calculator", showbug)
    bus.Publish("main:calculator", 20, 40)
    bus.Unsubscribe("main:calculator", showbug)
}

We use another implementation of eventbus (inspired by yours) where this issue was fixed: github.com/ispringteam/goeventbus (see copySubscriptions method and nextID field)

Feel free to adapt our solution or introduce another one ;)

alielbashir commented 2 years ago

Thanks for this, i've been struggling with this lib for real time updates with gRPC streaming and faced many bugs related to unsubscribe. Moving to your implementation seems to fix the problem