C
C#2y ago
yorimirus

❔ Synchronise an object across two threads?

Hello. I have an object that I use to wait listen for a message from a TcpConnection. However, while it is waiting, the connection could get lost. So I have a field for that called _connectionLost and I have an event that sets this field to false. The problem is that this event is fired from a different thread than the one that does the actual waiting. So when this field is changed, the other thread doesn't recognize that the value changed, so it will continue waiting for a message, even though it's pointless. When I look at it using a debugger, it still says _connectionLost is false. I have verified that the event gets properly fired and the field actually gets set. It's just that the other thread doesn't see that this change happened. What can I do to make sure the other thread sees the change? I tried locks, tokens and the volatile keyword. None seem to do anything.
22 Replies
yorimirus
yorimirusOP2y ago
Here is the code:
yorimirus
yorimirusOP2y ago
The weird thing is, the OnMessageReceived event is also run on a different thread than the one that actually waits, but this time, all the properties get properly set. The TCPConnection is abstracted away to an interface because I have a specific way I communicate through this connection. The WaitForResponse methods are the ones where the thread continuously checks for a message. While the events get fired on a different thread, that manages the actual connection. Anything else I can do? I'm out of ideas. Thanks in advance.
Sossenbinder
Sossenbinder2y ago
One thing which comes to my mind right away would be to use a TaskCompletionSource to await on, which you can fulfill in your event handler. That might make the whole looping logic a lot easier and you can probably even get away with fields altogether, keeping all state in the context of your method. Besides that, how does the event logic look like? Usually the event handlers run on the raising thread, unless specified otherwise I guess what you might need to investigate is whether you have multiple threads entering WaitForResponse, which could potentially reset your _gotResponse field even after it has been signalled as ready due to concurrent execution
yorimirus
yorimirusOP2y ago
yeah I throw an exception when that happens that's what the _waitingForResponse field is for TaskCompletionSource? Never heard of that. Will take a look at it. Thanks
Anton
Anton2y ago
while (!_gotResponse && (DateTime.Now - startTime).TotalSeconds < timeout)
Thread.Sleep(10);
while (!_gotResponse && (DateTime.Now - startTime).TotalSeconds < timeout)
Thread.Sleep(10);
yeah, never do this TaskCompletionSource is a classic for this scenario And also do use CancellationToken
yorimirus
yorimirusOP2y ago
What does a TaskCompletionSource do? I'm not quite sure what it does from the microsoft documentation.
Anton
Anton2y ago
it lets you treat events as tasks, that is, await until some event happens, without blocking the thread or sleeping
yorimirus
yorimirusOP2y ago
Ohh. So in the ConnectionLost event, I just set the TaskCompletionSource as completed?
Sossenbinder
Sossenbinder2y ago
It's a perfect match for event backed waiters Yeah
yorimirus
yorimirusOP2y ago
Never heard of this class, but sounds like the exact thing I need here
Sossenbinder
Sossenbinder2y ago
You should maybe also read up on synchronization primitives like Mutex or Semaphore, your if check is not entirely safe, multiple threads can still get through
yorimirus
yorimirusOP2y ago
Yeah I have used SemaphoreSlim a few times before. Might be a good idea to completely rewrite this class tbh.
Anton
Anton2y ago
also use TimeSpan for the timeouts
yorimirus
yorimirusOP2y ago
I wonder why the MessageReceived event works fine though -_- Okay I will go try it out, thanks a lot
Anton
Anton2y ago
Race conditions only show themselves once in 1000, it works until it doesn't
yorimirus
yorimirusOP2y ago
Actually it seems like I won't need any timespans now. If the events themselves set the task as complete, then I just need to use Task.WhenAny(task, Task.Delay(timeout * 1000)) to make it automatically abort.
Anton
Anton2y ago
I mean don't pass in the timeout as an int
yorimirus
yorimirusOP2y ago
oh
Anton
Anton2y ago
you have TimeSpan for this it's a value type too
yorimirus
yorimirusOP2y ago
yeah that's a good idea, no idea how I missed that Alright, managed to implement it the correct way. That still didn't work but after some further investigation, it wasn't because of multithreading but some completely different code entirely. Despite that, thanks a lot for your help. I managed to learn something new today thanks to you.
Anton
Anton2y ago
you're welcome
Accord
Accord2y ago
Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.
Want results from more Discord servers?
Add your server