❔ 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
Here is the code:
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.
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
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
yeah, never do this
TaskCompletionSource
is a classic for this scenario
And also do use CancellationToken
What does a TaskCompletionSource do? I'm not quite sure what it does from the microsoft documentation.
it lets you treat events as tasks, that is, await until some event happens, without blocking the thread or sleeping
Ohh. So in the ConnectionLost event, I just set the TaskCompletionSource as completed?
It's a perfect match for event backed waiters
Yeah
Never heard of this class, but sounds like the exact thing I need here
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
Yeah I have used SemaphoreSlim a few times before. Might be a good idea to completely rewrite this class tbh.
also use
TimeSpan
for the timeoutsI wonder why the MessageReceived event works fine though -_-
Okay I will go try it out, thanks a lot
Race conditions only show themselves once in 1000, it works until it doesn't
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.
I mean don't pass in the timeout as an int
oh
you have
TimeSpan
for this
it's a value type tooyeah 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.
you're welcome
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.