EscapeRoomsCommunity / escape_roomba

A Discord bot to bring ease and joy to the Escape Room community discord.
MIT License
1 stars 2 forks source link

Start of a thread manager #4

Closed egnor closed 4 years ago

egnor commented 4 years ago

Minor changes:

dcsan commented 4 years ago

this looks cool, is it working somewhere? do you have a test server you've deployed it on?

egnor commented 4 years ago

this looks cool, is it working somewhere? do you have a test server you've deployed it on?

Not quite yet but super soon! (I test it on "egnor's omelette" but a couple more things and I'll leave one running to play with)

egnor commented 4 years ago

this looks like handling race conditions... tricky stuff in python

It is, but (and I could spell this out better in the comments) these aren't true threads, they're async coroutines (which is what discord.py is built on). A lot of the logic is similar, but way simpler because the only concurrency points are "await" statements, everything else is single threaded, so there's way less to think about -- you don't have to think super hard about every possible way two pieces of code could line up, just ways the blocks between await statements could rearrange.

FYI, the race condition I'm defending against is one where multiple changes are made in quick succession to a message -- say, adding and then removing a reaction, or a quick edit just after another edit. We don't (can't) cache every single message (I want to be able to deal with reactions on arbitrarily old messages) so we go fetch the message after each event (the raw events don't give much info about the message, just the action itself and a message ID). The fetch operation is an independent asynchronous request (HTTP JSON), so you could hypothetically end up with this situation

    server - state changes on message M, sends a change notification
client - receives first change notification for M
client - sends first fetch request for M
    server - sends response to first fetch request for M, with current state
        --- fetch response gets stuffed up in the innertubes for a bit ---
    server - state changes again on M, sends a change notification
client - receives second change notification for message M (reordered before first fetch response)
client - sends second fetch request for M
    server - sends response to second fetch request for M, with updated state
        --- second fetch response sails right through, waving at the first response as it passes ---
client - receives SECOND fetch response for M with updated state (reordered before first fetch response)
client - (finally!) receives FIRST fetch response for M with previous state
client - now thinks the old state is current (until another update of that message)

Since the fetch requests can all run over their own HTTP channels this reordering is perfectly possible due to random network glitches or Discord's server farm handling things differently or whatever. You decide the odds of this happening, but it's not totally implausible especially with rapid fire reaction / undo-reaction clicks.

So that's why I handle all message changes by going through a serialized-for-each-message fetch/update process, even for updates that could be handled immediately based on the change event. Probably some digest of the above should get included in the code comments!

(Simpler bots either can react to all changes immediately without needing any more context than the raw event, or rely on the cache having enough coverage to avoid needing fetch requests, or just deal with things getting out of order occasionally and people saying "huh, that's weird, guess I'll retoggle the emoji -- oh that fixed it hmm".)