search

bwmarrin / discordgo

(Golang) Go bindings for Discord
BSD 3-Clause "New" or "Revised" License
5.05k stars 802 forks source link

State tracking stomps on objects #1363

Open comstud opened 1 year ago

comstud commented 1 year ago

The tracking in state.go uses a lock to protect against concurrent modifications to its own structs. However, when updating cached objects, it overwrites them in place instead of caching a new object. It is possible for these objects to have been retrieved previously and their memory concurrently being accessed while the state tracking is writing to their memory location during updates.

An example from memberAdd():

m, ok := members[member.User.ID]
[...]
*m = *member

This 'm' may be in use elsewhere, being accessed, while this is replacing it.. either via previously having retrieved it, or having retrieved the Guild object and are iterating the Guild.Members, etc.

I don't see an easy way to fix this other than requiring one to lock the State themselves any time they want to access any part of the objects they've retrieved from State. Other solutions seem to require a bit of an overhaul of the state code. Caching a new Member would require also caching a new Guild... and would require a lot of looping through the Guild.Members array.

comstud commented 1 year ago

I guess another solution would always to return deep copies of objects when retrieving things from State. That potentially has a high cost for Guild... though you may end up with a similar cost even when fixing it differently.