0

Im making a server for a new game proyect and im using C# (.NET Framework v4.5) for both client and server.

Im currently using something like this to manage my players.

new Thread(() =>
{
   connection.ListenAndSend();
}).Start();

public void ListenAndSend()
{
    while(working)
    {
        if(someThingToRead)
        {
        //Listening logic here

        //Here i call my event for dataReceived with the data i just read.
        }
        else if (queue.Count > 0)
        {
            Send(queue.Dequeue().Data); //Send data from the Queue
        }
    }

The use of a Queue is because i should not access the stream from different threads (it throws InvalidOperationException i think) so i implemented the queue to send data from the same thread.

I think this is mostly OK but im worried about the events.

The events are firing and working fine but i have not tested yet with multiple clients so...

  • Is the event being executed in the same thread that the listener thread?
  • Could i have problems with fields being modified by multiple threads at the same time? (Im more experienced in java for example and i remember something about Syncronize interface?)
  • Its not done right now but later its possible that some event will end up calling the send method so adding data to the send Queue (But this should not be an issue right?)

This game will never be for a large number of players (probably between 2 and 8) if that matters.

Is there some serious problem im not seeing?

As im using the Queue im thinking it does not matter what thread do add the data but will my listening be stopped while it is doing the event code? (If its really on the same thread) and while that could not really be an issue will it be simultaneously accesing fields from multiple threads?

UPDATE:

Can i make all of this just with async/await?? i cant see how to make the listen loop that way but if its really possible please expand in an answer.

Also, this answer says the opposite.

UPDATE 2:

Now that i have tested my code and i know its working fine.

As pointed out in comments i could have done this using async/await with a lot less threads, but:

In which it would be better to use async/await instead of threads, keep in mind my game server should be able to take as much cpu/memory as needed and its fine if it does (Of course the threads are calling sleep when not doing job). And i would like my server to run the closer to real time as possible.

I also would like to point, the first time i made it to start processing my logic the serializing of the game map (around 60k objects) took around 600ms (and that will be for each client) so i ended up moving the serialize job to the socket thread.

With this in mind, who thinks it would be better to use Async/Await instead of new Threads

PD: Im only talking about my server, the game client is indeed using async/await to communicate with the server.

Nanoc
  • 2,381
  • 1
  • 20
  • 35
  • Are you using a `NetworkStream`? Because it's not true that you cannot access it from multiple threads - just as long as you don't have two threads both reading at the same time, or two threads both writing at the same time. – C.Evenhuis Oct 18 '17 at 11:18
  • You execute `ListenAndSend` method as a background thread. Your event handler will be executed in the same thread (and will block further receiving) unles you use https://msdn.microsoft.com/en-us/library/system.windows.threading.dispatcher(v=vs.110).aspx. – Oguz Ozgul Oct 18 '17 at 11:20
  • @C.Evenhuis Yes im using NetworkStream and im pretty sure that i was getting a exception when calling Write from another thread (although its pretty possible the other thread was calling read) but even if i can should i? – Nanoc Oct 18 '17 at 11:21
  • @Nanoc you can concurrently read on one thread and write on another - just as long as you don't have _two threads both reading at the same time_, or _two threads both writing at the same time_. – C.Evenhuis Oct 18 '17 at 11:22
  • @C.Evenhuis i will have to check that again – Nanoc Oct 18 '17 at 11:23
  • @Nanoc You don't need a background thread, events or locking. All supported .NET versions provide asynchronous operations, async/await and concurrent collections like ConcurrentQueue. Which class are you using? For example TcpClient offers async versions of all its methods. Stream-derived classes all offer async versions of their methods – Panagiotis Kanavos Oct 18 '17 at 11:23
  • @Nanoc as for queueing, [ConcurrentQueue](https://msdn.microsoft.com/en-us/library/dd267265(v=vs.110).aspx) allows multiple threads to read *and* write to it without locking – Panagiotis Kanavos Oct 18 '17 at 11:23
  • @Nanoc you can even get rid of queuing if you use eg [ActionBlock](https://msdn.microsoft.com/en-us/library/hh194684(v=vs.110).aspx) to send messages, as it includes a built-in buffer – Panagiotis Kanavos Oct 18 '17 at 11:25
  • @PanagiotisKanavos do you mean i can make all the logic just using async/await?? i thought it was just to avoid blocking the main thread but i really cant imagine how to make the reading loop for multiple players that way – Nanoc Oct 18 '17 at 11:25
  • @Nanoc what avoids blocking are the asynchronous calls. `async/await` allows you to *await* and continue execution once those the async operation completes. Events, callbacks and raw threads aren't needed. – Panagiotis Kanavos Oct 18 '17 at 11:28
  • @Nanoc `async/await` won't make a method asynchronous by magic, so your listening loop *may* have to run inside another task. That's not a hard requirement though - if you read a message and immediatelly post it to a queue or block for processing, the impact will be minimal. – Panagiotis Kanavos Oct 18 '17 at 11:29
  • @Nanoc the answer you linked to doesn't say that you *can't* use async/await. – Panagiotis Kanavos Oct 18 '17 at 11:31
  • @PanagiotisKanavos i think i got it but will it be better than implementing a dispatcher to send the events? and the ConcurrentQueue you mentioned, changing all to async/await is way more work – Nanoc Oct 18 '17 at 11:32
  • @Nanoc which classes are you actually using? TcpClient, TcpListener don't have events. What is "actually more work" since you don't have any code to begin with? How can it be *more* when, eg instead of `Connect` you call `ConnectAsync`. Anyway, sockets have no events, no dispatchers. Dispatchers *were* used to avoid blocking the UI. They were replaced by `async/await`, the `IProgress< T>` interface and similar mechanisms. – Panagiotis Kanavos Oct 18 '17 at 11:35
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/156997/discussion-between-nanoc-and-panagiotis-kanavos). – Nanoc Oct 18 '17 at 11:35
  • @Nanoc please update the question with the classes you actually use. It's hard to post a concrete example when one has to guess at the actual code – Panagiotis Kanavos Oct 18 '17 at 11:36

1 Answers1

2

You have 2 options , ether lock the variable (which is not recommended) How to lock a variable used in multiple threads

or use dispatcher for calling a method from another thread Using the C# Dispatcher

if you are not using WPF, you might take a look to this link : how do I convert wpf dispatcher to winforms

Ehsan Zargar Ershadi
  • 24,115
  • 17
  • 65
  • 95
  • There are no "methods from another thread". Threads execute, they don't own methods. The dispatcher *was* needed to update UI controls. All supported version of .NET though support `async/await` nowadays, so there's no need to use a dispatcher – Panagiotis Kanavos Oct 18 '17 at 11:21