• Advertisement
Sign in to follow this  

Python how to stop 2 threads from modifying a variable at the same time

This topic is 584 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I just found a bug on the multiplayer RPG i'm developing in Python and i'm not quite sure how to get around it.
From what I can see, 2 threads are acting upon a variable at the same time causing an item duplication bug.

In this game when you die, the only thing you lose is what is in your "hand" (such as a magic orb) and it drops to the ground. When you switch the item in your hand to your inventory at the same time you die the item drops to the ground and goes in to your inventory.
 
The server listen thread instance for a client and receives this packet "A:ti-5: (action:transfer_to_inventory-spot#5)

if (data[2:4] == "ti"):
    temp = self.player.hand
    self.player.hand = self.player.inventory[int(data[5:len(data)])]
    self.player.inventory[int(data[5:len(data)])] = temp

Which is the client signalling to the server that he is swapping an item in his "hand" to a selected inventory space.

 

When the player receives fatal damage (server side) the item in the players "hand" is made equal to 0 (null) and the item is dropped to the ground.
 

client[target].player.hp -= dmg

       if client[target].player.hp <= 0: #If player is dead..
             if client[target].player.hand > 0: #".. and has something in his hand, drop the item
                 tempItem = Item()
                 tempItem.idNum = client[target].player.hand
                 tempItem.map = client[target].player.map
                 tempItem.mapX = client[target].player.mapX
                 tempItem.mapY = client[target].player.mapY
                 tempItem.isThere = True
                 itemsOnGround.append(tempItem)
                 client[target].player.hand = 0

I'm not sure how to do this, but the logic that makes sense to me is to "lock" important variables while they are in use and un-lock them when the action is completed. 
My concern now is that if this bug is possible, there are likely more. Like equipping armor/weapon while dying, or something similar.

Any advice would be appreciated!

 

Share this post


Link to post
Share on other sites
Advertisement

Thank you for pointing me in the right direction.
So would I be right in assuming the purpose of a thread lock is to give 1 thread the ability to complete tasks before releasing the processor so other threads can execute their tasks?
If so, in this way I can use a thread lock to stop all other processes from executing long enough for a thread that needs to execute an important change to finish before releasing the lock?

Share this post


Link to post
Share on other sites

I certainly would not look to locks at the _first_ answer. They might be the answer, but there's a bigger question here first (the one you had as essentially a footnote): why is this happening in the first place?

 

Why are two threads even able to modify the same piece of data? You mention a "listen thread" but then there's also some other main thread that's doing work on the player. That's generally not good design, and it's one of the evils of multithreading that makes it _seem_ harder than it really is.

 

The golden rule here: every object should have a clear and unambiguous owner, and only that owner should ever be able to operate on the object. In other words, decide if the main server thread or the "listen" thread owns the player, and only ever ever let that thread modify the player. If another thread needs the player then it will need to obtain a read-only copy or it will need to send messages to the owning thread asking for the mutation to be made.

 

Is that possible in every case? No. But you should strive for it and fall back to lock-based threading only when you truly must.

 

It's safer, easier, _and faster_.

Share this post


Link to post
Share on other sites
I originally designed the server with that idea in mind but ran in to an issue with the connection.recv() method halting the thread until it received data, which meant at that point users only got game state updates after they sent data to the server.

As a solution I created a client listener thread and a thread to update the game state. The listener thread handled the hand to inventory switch request and the game state thread updated the hand when it determined the player died.
I think this is one of the uncommon instances where the game state thread updates a variable rather than just sending it back to the clients.
Thank you for your explanation! I now have a much better idea on how to structure my code in a multithreaded program.

Share this post


Link to post
Share on other sites

I originally designed the server with that idea in mind but ran in to an issue with the connection.recv() method halting the thread until it received data
You may want to check the "select" module in the standard library, which also has a timeout option.

There are some OS-specific limitations on it though, so it may not work for you.

Share this post


Link to post
Share on other sites

I originally designed the server with that idea in mind but ran in to an issue with the connection.recv() method halting the thread until it received data, which meant at that point users only got game state updates after they sent data to the server.
 

 

Look into "event-driven IO" and frameworks like Twisted for Python (https://twistedmatrix.com/trac/).

 

That's not a suggestion. That's completely The Answer(tm).

 

I assume by your comment that you have one "listen" thread per client - that's super ungood. That was a common approach back in the 90's when Python and Java were new and nobody really understood threading yet.

 

IO is purely event driven at the highest conceptual levels. You never want to "wait" for IO. You want to set up a continuation for when IO is available, e.g. register an event handler for when IO is ready. Frameworks like Twisted provide that for Python. Very new versions of Python also have various bits of that functionality built in (https://docs.python.org/3/library/asyncio-task.html).

 

Taken to its conclusion you should remove all threads other than the main thread. Python can't make good use of multiple hardware cores anyway because of its GIL (Global Interpreter Lock) so there's no good reason to use threads in Python. Getting rid of threads will hence be purely a simplification of your code and a removal of a source of very hard bugs. For concurrency, use coroutines and tasks and events and promises ("deferreds" in Twisted nomenclature) instead of threads.

 

I can 100% promise you with professional authority on the subject that real well-known MMOs written using Python use Twisted and do not use threads (using multi-process architectures for hardware scalability, since that avoids the Python GIL).

Share this post


Link to post
Share on other sites

Thank you very much Sean I'll definitely have to look in to event-driven IO.
To be honest I haven't done a project like this since the early 2000s and haven't learned much about multithreading or networking since then, I just decided one day to make this project and 100 hrs & 1 month later here I am lol.
I appreciate the advice you have given me, it seems that I have a fair bit to catch up on and learn.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement