From 480fa533a28b1d79404136312bc5d46e14a98434 Mon Sep 17 00:00:00 2001 From: Tyler Stiene Date: Wed, 7 Apr 2021 01:24:17 -0400 Subject: [PATCH] fix race data related to reading bridge state --- Makefile | 3 +++ bridge.go | 12 ++++++++++++ discord-handlers.go | 16 +++++++++++++--- discord.go | 14 +++++++++----- main.go | 4 +++- mumble-handlers.go | 5 +++-- 6 files changed, 43 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 221c93c..5d9d6cd 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,9 @@ mumble-discord-bridge: $(GOFILES) dev: $(GOFILES) goreleaser build --skip-validate --rm-dist && sudo ./dist/mumble-discord-bridge_linux_amd64/mumble-discord-bridge +dev-race: $(GOFILES) + go run -race *.go + dev-profile: $(GOFILES) goreleaser build --skip-validate --rm-dist && sudo ./dist/mumble-discord-bridge_linux_amd64/mumble-discord-bridge -cpuprofile cpu.prof diff --git a/bridge.go b/bridge.go index 571ed0c..5703a60 100644 --- a/bridge.go +++ b/bridge.go @@ -35,6 +35,9 @@ type BridgeState struct { // Wait for bridge to exit cleanly WaitExit *sync.WaitGroup + // Bridge State Mutex + BridgeMutex sync.Mutex + // Bridge connection Connected bool @@ -189,7 +192,9 @@ func (b *BridgeState) startBridge() { } }() + b.BridgeMutex.Lock() b.Connected = true + b.BridgeMutex.Unlock() // Hold until cancelled or external die request select { @@ -200,7 +205,10 @@ func (b *BridgeState) startBridge() { cancel() } + b.BridgeMutex.Lock() b.Connected = false + b.BridgeMutex.Unlock() + wg.Wait() log.Println("Terminating Bridge") b.MumbleUsersMutex.Lock() @@ -221,6 +229,7 @@ func (b *BridgeState) discordStatusUpdate() { b.DiscordSession.UpdateListeningStatus("an error pinging mumble") } else { b.MumbleUsersMutex.Lock() + b.BridgeMutex.Lock() b.MumbleUserCount = resp.ConnectedUsers if b.Connected { b.MumbleUserCount = b.MumbleUserCount - 1 @@ -234,6 +243,7 @@ func (b *BridgeState) discordStatusUpdate() { status = fmt.Sprintf("%v users in Mumble\n", b.MumbleUserCount) } } + b.BridgeMutex.Unlock() b.MumbleUsersMutex.Unlock() b.DiscordSession.UpdateListeningStatus(status) } @@ -257,6 +267,7 @@ func (b *BridgeState) AutoBridge() { b.MumbleUsersMutex.Lock() b.DiscordUsersMutex.Lock() + b.BridgeMutex.Lock() if !b.Connected && b.MumbleUserCount > 0 && len(b.DiscordUsers) > 0 { log.Println("users detected in mumble and discord, bridging") @@ -267,6 +278,7 @@ func (b *BridgeState) AutoBridge() { b.BridgeDie <- true } + b.BridgeMutex.Unlock() b.MumbleUsersMutex.Unlock() b.DiscordUsersMutex.Unlock() } diff --git a/discord-handlers.go b/discord-handlers.go index 58f3ac2..e0e3083 100644 --- a/discord-handlers.go +++ b/discord-handlers.go @@ -49,11 +49,13 @@ func (l *DiscordListener) guildCreate(s *discordgo.Session, event *discordgo.Gui l.Bridge.DiscordUsersMutex.Unlock() // If connected to mumble inform users of Discord users + l.Bridge.BridgeMutex.Lock() if l.Bridge.Connected && !l.Bridge.BridgeConfig.MumbleDisableText { l.Bridge.MumbleClient.Do(func() { l.Bridge.MumbleClient.Self.Channel.Send(fmt.Sprintf("%v has joined Discord\n", u.Username), false) }) } + l.Bridge.BridgeMutex.Unlock() } } @@ -85,10 +87,14 @@ func (l *DiscordListener) messageCreate(s *discordgo.Session, m *discordgo.Messa return } + l.Bridge.BridgeMutex.Lock() + bridgeConnected := l.Bridge.Connected + l.Bridge.BridgeMutex.Unlock() + if strings.HasPrefix(m.Content, prefix+" link") { // Look for the message sender in that guild's current voice states. for _, vs := range g.VoiceStates { - if l.Bridge.Connected { + if bridgeConnected { l.Bridge.DiscordSession.ChannelMessageSend(m.ChannelID, "Bridge already running, unlink first") return } @@ -103,7 +109,7 @@ func (l *DiscordListener) messageCreate(s *discordgo.Session, m *discordgo.Messa if strings.HasPrefix(m.Content, prefix+" unlink") { // Look for the message sender in that guild's current voice states. - if !l.Bridge.Connected { + if !bridgeConnected { l.Bridge.DiscordSession.ChannelMessageSend(m.ChannelID, "Bridge is not currently running") return } @@ -118,7 +124,7 @@ func (l *DiscordListener) messageCreate(s *discordgo.Session, m *discordgo.Messa if strings.HasPrefix(m.Content, prefix+" refresh") { // Look for the message sender in that guild's current voice states. - if !l.Bridge.Connected { + if !bridgeConnected { l.Bridge.DiscordSession.ChannelMessageSend(m.ChannelID, "Bridge is not currently running") return } @@ -195,11 +201,13 @@ func (l *DiscordListener) voiceUpdate(s *discordgo.Session, event *discordgo.Voi seen: true, dm: dm, } + l.Bridge.BridgeMutex.Lock() if l.Bridge.Connected && !l.Bridge.BridgeConfig.MumbleDisableText { l.Bridge.MumbleClient.Do(func() { l.Bridge.MumbleClient.Self.Channel.Send(fmt.Sprintf("%v has joined Discord\n", u.Username), false) }) } + l.Bridge.BridgeMutex.Unlock() } else { du := l.Bridge.DiscordUsers[vs.UserID] du.seen = true @@ -213,11 +221,13 @@ func (l *DiscordListener) voiceUpdate(s *discordgo.Session, event *discordgo.Voi for id := range l.Bridge.DiscordUsers { if !l.Bridge.DiscordUsers[id].seen { log.Println("User left Discord channel " + l.Bridge.DiscordUsers[id].username) + l.Bridge.BridgeMutex.Lock() if l.Bridge.Connected && !l.Bridge.BridgeConfig.MumbleDisableText { l.Bridge.MumbleClient.Do(func() { l.Bridge.MumbleClient.Self.Channel.Send(fmt.Sprintf("%v has left Discord channel\n", l.Bridge.DiscordUsers[id].username), false) }) } + l.Bridge.BridgeMutex.Unlock() delete(l.Bridge.DiscordUsers, id) } } diff --git a/discord.go b/discord.go index 3fa8518..4268780 100644 --- a/discord.go +++ b/discord.go @@ -23,9 +23,8 @@ type fromDiscord struct { type DiscordDuplex struct { Bridge *BridgeState - discordMutex sync.Mutex - discordMixerMutex sync.Mutex - fromDiscordMap map[uint32]fromDiscord + discordMutex sync.Mutex + fromDiscordMap map[uint32]fromDiscord } // OnError gets called by dgvoice when an error is encountered. @@ -87,6 +86,7 @@ func (dd *DiscordDuplex) discordSendPCM(ctx context.Context, wg *sync.WaitGroup, continue } + dd.Bridge.DiscordVoice.RWMutex.RLock() if !dd.Bridge.DiscordVoice.Ready || dd.Bridge.DiscordVoice.OpusSend == nil { if lastReady { OnError(fmt.Sprintf("Discordgo not ready for opus packets. %+v : %+v", dd.Bridge.DiscordVoice.Ready, dd.Bridge.DiscordVoice.OpusSend), nil) @@ -96,13 +96,15 @@ func (dd *DiscordDuplex) discordSendPCM(ctx context.Context, wg *sync.WaitGroup, }) lastReady = false } - continue } else if !lastReady { fmt.Println("Discordgo ready to send opus packets") lastReady = true readyTimeout.Stop() + } else { + dd.Bridge.DiscordVoice.OpusSend <- opus } - dd.Bridge.DiscordVoice.OpusSend <- opus + dd.Bridge.DiscordVoice.RWMutex.RUnlock() + } else { if streaming { dd.Bridge.DiscordVoice.Speaking(false) @@ -123,6 +125,7 @@ func (dd *DiscordDuplex) discordReceivePCM(ctx context.Context, wg *sync.WaitGro wg.Add(1) for { + dd.Bridge.DiscordVoice.RWMutex.RLock() if !dd.Bridge.DiscordVoice.Ready || dd.Bridge.DiscordVoice.OpusRecv == nil { if lastReady { OnError(fmt.Sprintf("Discordgo not to receive opus packets. %+v : %+v", dd.Bridge.DiscordVoice.Ready, dd.Bridge.DiscordVoice.OpusSend), nil) @@ -138,6 +141,7 @@ func (dd *DiscordDuplex) discordReceivePCM(ctx context.Context, wg *sync.WaitGro lastReady = true readyTimeout.Stop() } + dd.Bridge.DiscordVoice.RWMutex.RUnlock() var ok bool var p *discordgo.Packet diff --git a/main.go b/main.go index e06bef2..00e720b 100644 --- a/main.go +++ b/main.go @@ -40,7 +40,7 @@ func main() { mumblePassword := flag.String("mumble-password", lookupEnvOrString("MUMBLE_PASSWORD", ""), "MUMBLE_PASSWORD, mumble password, optional") mumbleInsecure := flag.Bool("mumble-insecure", lookupEnvOrBool("MUMBLE_INSECURE", false), " MUMBLE_INSECURE, mumble insecure, optional") mumbleCertificate := flag.String("mumble-certificate", lookupEnvOrString("MUMBLE_CERTIFICATE", ""), "MUMBLE_CERTIFICATE, client certificate to use when connecting to the Mumble server") - mumbleChannel := flag.String("mumble-channel", lookupEnvOrString("MUMBLE_CHANNEL", ""), "MUMBLE_CHANNEL, mumble channel to start in, using '/' to seperate nested channels, optional") + mumbleChannel := flag.String("mumble-channel", lookupEnvOrString("MUMBLE_CHANNEL", ""), "MUMBLE_CHANNEL, mumble channel to start in, using '/' to separate nested channels, optional") mumbleDisableText := flag.Bool("mumble-disable-text", lookupEnvOrBool("MUMBLE_DISABLE_TEXT", false), "MUMBLE_DISABLE_TEXT, disable sending text to mumble, (default false)") discordToken := flag.String("discord-token", lookupEnvOrString("DISCORD_TOKEN", ""), "DISCORD_TOKEN, discord bot token, required") discordGID := flag.String("discord-gid", lookupEnvOrString("DISCORD_GID", ""), "DISCORD_GID, discord gid, required") @@ -197,9 +197,11 @@ func main() { log.Println("OS Signal. Bot shutting down") // Wait or the bridge to exit cleanly + Bridge.BridgeMutex.Lock() if Bridge.Connected { //TODO BridgeDie occasionally panics on send to closed channel Bridge.BridgeDie <- true Bridge.WaitExit.Wait() } + Bridge.BridgeMutex.Unlock() } diff --git a/mumble-handlers.go b/mumble-handlers.go index 6350c68..3dc58df 100644 --- a/mumble-handlers.go +++ b/mumble-handlers.go @@ -43,22 +43,23 @@ func (l *MumbleListener) mumbleUserChange(e *gumble.UserChangeEvent) { e.User.Send("Mumble-Discord-Bridge v" + version) // Tell the user who is connected to discord + l.Bridge.DiscordUsersMutex.Lock() if len(l.Bridge.DiscordUsers) == 0 { e.User.Send("No users connected to Discord") } else { s := "Connected to Discord: " arr := []string{} - l.Bridge.DiscordUsersMutex.Lock() for u := range l.Bridge.DiscordUsers { arr = append(arr, l.Bridge.DiscordUsers[u].username) } s = s + strings.Join(arr[:], ",") - l.Bridge.DiscordUsersMutex.Unlock() e.User.Send(s) } + l.Bridge.DiscordUsersMutex.Unlock() + } // Send discord a notice