data race

[dajohi]

WARNING: DATA RACE            
Write at 0x00c0009cf358 by goroutine 29:                    
  github.com/bwmarrin/discordgo.(*State).MemberAdd()        
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/state.go:311 +0x2b8                       
  github.com/bwmarrin/discordgo.(*State).OnInterface()      
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/state.go:937 +0x1119                      
  github.com/bwmarrin/discordgo.(*Session).onInterface()    
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/event.go:236 +0x1be                       
  github.com/bwmarrin/discordgo.(*Session).handleEvent()    
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/event.go:194 +0x96                        
  github.com/bwmarrin/discordgo.(*Session).onEvent()        
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/wsapi.go:562 +0x18ce                      
  github.com/bwmarrin/discordgo.(*Session).listen()         
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/wsapi.go:251 +0xaf                        

Previous read at 0x00c0009cf358 by goroutine 67:            
  github.com/42wim/matterbridge/bridge/discord.(*Bdiscord).memberAdd()                                               
      /home/matterbridge/.go/src/github.com/42wim/matterbridge/bridge/discord/handlers.go:135 +0x8f                  
  github.com/42wim/matterbridge/bridge/discord.(*Bdiscord).memberAdd-fm()                                            
      /home/matterbridge/.go/src/github.com/42wim/matterbridge/bridge/discord/handlers.go:130 +0x55                  
  github.com/bwmarrin/discordgo.guildMemberAddEventHandler.Handle()                                                  
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/eventhandlers.go:317 +0x69                

Goroutine 29 (running) created at:                          
  github.com/bwmarrin/discordgo.(*Session).Open()           
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/wsapi.go:203 +0x1319                      
  github.com/bwmarrin/discordgo.(*Session).reconnect()      
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/wsapi.go:796 +0x128                       
  github.com/bwmarrin/discordgo.(*Session).listen()         
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/wsapi.go:239 +0x38b                       

Goroutine 67 (finished) created at:                         
  github.com/bwmarrin/discordgo.(*Session).handle()         
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/event.go:171 +0x1df                       
  github.com/bwmarrin/discordgo.(*Session).handleEvent()    
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/event.go:200 +0x103                       
  github.com/bwmarrin/discordgo.(*Session).onEvent()        
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/wsapi.go:562 +0x18ce                      
  github.com/bwmarrin/discordgo.(*Session).listen()         
      /home/matterbridge/.go/pkg/mod/github.com/bwmarrin/discordgo@v0.19.0/wsapi.go:251 +0xaf                        
==================  

[bwmarrin]

Also see, 42wim/matterbridge#815

[horgh]

Would replacing the Member in MemberAdd() work? As opposed to updating the one we have in place. e.g.:

diff --git a/state.go b/state.go
index e6f08c7..91cb7e0 100644
--- a/state.go
+++ b/state.go
@@ -308,7 +308,8 @@ func (s *State) MemberAdd(member *Member) error {
                if member.JoinedAt == "" {
                        member.JoinedAt = m.JoinedAt
                }
-               *m = *member
+               members[member.User.ID] = member
+               // TODO update guild.Members
        }

        return nil

[bwmarrin]

So there's a couple of issues here, which took me a few mins to figure out since it's been so long, the traces here are pointing to the wrong lines. Sorry it's been this long before I could get to this.

In the race from the matterbridge issue 42wim/matterbridge#815

• Discord Sent a New User
• Both DiscordGo state and Matterbridge State has handlers for this.
• DiscordGo state got it faster, added the member.
• Matterbridge got the same member pointer and started to look at it.
• Right then, a PresenceUpdate came in for that member and the DiscordGo state updated that members information.

Since all these actions ended up looking at a pointer to the same item - we have a data race. To fix this, the Presence update in DiscordGo should make a new copy of Member and write a pointer of that back into the State as @horgh recommends.

Which has the side effect that in the above case, by the time Matterbridge started looking at the new member, it's data will have already been updated/changed in DiscordGo state.

Then we have the data race trace in this ticket right here, which is pretty much the same it's just getting caught at different points. I'll submit a proposed PR later today/tomorrow for this.

[bwmarrin]

I've created a branch for this with a partial fix, we need to figure out how to properly update guild.Members and I haven't thought up a good way yet. Because there would be data race issues there as well, I believe.

Might be that we have to make a copy of it too so that's probably what I'll do for now, and maybe we can come up with a more efficient way later.

[bwmarrin]

After reviewing this a bit more, I think the problem is a big bigger than just MemberAdd. I'm going to close this in favour of #731 in which I'll try to find a more global fix to the problem.