My lock()s don't appear to be working.

Started by
1 comment, last by belgreth 11 years, 9 months ago
I was fairly sure I had my locks correct until I had my network thread update the direction of players during state changes. So whenever players turn they get distorted for a bit when changing directions. My setup is briefly as follows:

[source lang="csharp"]public class UnitContainer
{
public static readonly object OtherPlayersLocker = new object(); // i tried non-static as well
List<Player> otherPlayers;
// ...
}[/source]

I pass the unitContainer to all the classes that contain threads so they can access the player list and any time a thread accesses and/or modifies player data they call the lock (such as)

[source lang="csharp"]lock (UnitContainer.OtherPlayersLocker)
{
unitContainer.otherPlayers[index].Direction = msg.messageList.direction;
unitContainer.otherPlayers[index].Direction.Normalize();
}[/source]

The problem goes away when I normalize the direction before setting it to the otherplayer. But regardless, the way I have it set up, it shouldn't matter if I do or not.

My guess is either the Draw call is drawing while the world variable is being updated, or the world variable is being updated after direction was set but before it was normalized. However if everything is in that lock (including draw call)..how are the two threads modifying/accessing the player data at the same time?
Advertisement
The problem with bare locks is that it is extremely hard to prove that you don't use them incorrectly somewhere.

It is a good idea to do only the minimal work required during the lock. Normalising the vector can be safely done outside the lock. You generally do not want to be holding a long during a long operation (e.g. drawing the scene).

Sometimes, a simpler solution is better. You may not need to thread your network - a non-blocking poll might be a better option. Another option would be to have the networking thread add the state update information to a thread-safe queue, which the main loop drains once per frame.
I decided to put the received message processing back into the main thread. I am unsure why I originally had a separate thread that only processed incoming messages from the server. I'm also unsure what I was thinking when I put the whole draw call into the lock when I could've just held the lock long enough to copy the world variable into a temp matrix and then call draw...

I already have 2 threads for networking, one that only listens, and stuffs the packet into a list and immediately goes back to listening and another thread that just decodes the packets into messages. My locks for those things seem to be working as I haven't had any race conditions or out of index errors yet. In fact there's no need for the second thread as well, I can stuff that into the main thread too.

I agree, keeping it simple is best. I am not aware of polling so I'll definitely be looking into that. Thanks for the reply rip-off.



EDIT: Well after putting the message processor in the main thread I was still having the same issue so apparently the locks weren't the problem to begin with! The direction wasn't getting normalized for the player when calling

unitContainer.otherPlayers[index].Direction.Normalize();

It works when I did:

unitContainer.otherplayers[index].Direction = Vector3.Normalize(unitContainer.otherplayers[index].Direction);

If anyone has any idea why the first one didn't work I'd love to know.

This topic is closed to new replies.

Advertisement