How can I make sure my task is aware of a flag set on the main thread?

Question

I'm using the Unity game engine to do some networking, and I want to use the built-in language features rather than rely on a third-party asset or Unity API. I've come up with this component, which attaches to a GameObject. When the game object is enabled, it should send UDP packets to a specified destination.

That part works. The trouble I'm having is with this sending flag:

using System;
using System.IO;
using System.Net;
using System.Net.Sockets;
using System.Threading.Tasks;
using UnityEngine;

namespace UdpTest
{
    public class DummySender : MonoBehaviour
    {
        public string destinationAddress = "127.0.0.1";
        public int port = 55555;

        UdpClient client;
        bool sending;
        IPEndPoint endpoint;

        private void Awake()
        {
            client = new UdpClient();
            try
            {
                endpoint = new IPEndPoint(IPAddress.Parse(destinationAddress), port);
                client.Client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.SendTimeout, 5000);
                client.Connect(endpoint);
            }
            catch (Exception e)
            {
                Debug.LogError("Couldn't set up dummy sender: " + e.Message, gameObject);
                gameObject.SetActive(false);
            }
        }

        private void OnEnable()
        {
            sending = true;
            new Task(() =>
            {
                try
                {
                    Debug.Log("Ready to send");
                    while (sending)
                    {
                        var bytes = System.Text.Encoding.ASCII.GetBytes(DateTime.Now.Millisecond.ToString() + "\n");
                        client.Send(bytes, bytes.Length);
                    }
                }
                catch (Exception e)
                {
                    Debug.LogError("Dummy send failed: " + e.Message);
                }
            }).Start();
        }

        private void OnDisable()
        {
            sending = false;
        }
    }
}

The issue I'm having is that I can't make it stop, once the task with the loop is running.

My plan was to make it stop when the component is disabled, by setting the sending flag to false. I realize that setting the flag to false happens on a different thread. My expectation was that, yes, the loop may run a few more times between the sending flag getting set to false and the task accessing its value, but that is fine for my purposes.

However, what is actually happening is that the task doesn't seem to ever become aware that the value of sending has changed. It remains true, The loop runs indefinitely, and my receiver doesn't stop receiving packets until I kill the program.

Why is this? I'm a bit new to low-level tasks/threading, but I understand that there's no guarantee that the worker task would be aware of the change immediately. I would have expected it to update eventually. But in my case, although I can verify that the sending flag has been set to false on the main thread, the loop in the task never stops.

If I had to guess, this seems like the same kind of mistake as you get when you copy a value and change the original (i.e. a by reference/by value mistake). Does the task actually have a copy of sending and not a reference to the class instance field? Or am I missing something else?

I know there are other strategies to achieve UDP communications, such as using the Async variants but, for the sake of learning, I would like to know what I am doing wrong here and how to fix it without changing tack completely.

Would be really nice if someone would point out why this question is anti-valuable as well, if you are going to vote it down, as I am apparently clueless on that as well. — Joseph, Apr 04 '20 at 20:29
Try declaring the `sending` variable as [`volatile`](https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/volatile). Like this: `volatile bool sending;` — Theodor Zoulias, Apr 04 '20 at 21:11
@TheodorZoulias according to this answer: https://stackoverflow.com/questions/29411961/c-sharp-and-thread-safety-of-a-bool, adding volatile keyword doesn't guarantee the field to be thread-safe. — Arutyun Enfendzhyan, Apr 04 '20 at 21:15
@ArutyunEnfendzhyan this case has less to do with thread-safety, and more to do with visibility. One thread is not seeing a change made to a variable by another thread. The topic of visibility is extremely obscure and complex, and the details are hairy and sometimes scary. Here are two related questions: [Q1](https://stackoverflow.com/questions/38166019/c-sharp-bool-is-atomic-why-is-volatile-valid), [Q2](https://stackoverflow.com/questions/516863/does-interlocked-provide-visibility-in-all-threads). — Theodor Zoulias, Apr 04 '20 at 22:08
@TheodorZoulias that's exactly what I meant by thread-safe. Here is the definition of "thread-safe" - Thread-safe code only manipulates shared data structures in a manner that ensures that all threads behave properly and fulfill their design specifications without unintended interaction. — Arutyun Enfendzhyan, Apr 05 '20 at 08:22
@ArutyunEnfendzhyan for me "thread-safety" is about shared state not being corrupted when manipulated by multiple threads. In this particular case declaring the field as `volatile` seems good enough, since this field is queried continuously in a loop. The `CancellationTokenSource` class uses `volatile` fields too [internally](https://referencesource.microsoft.com/mscorlib/system/threading/CancellationTokenSource.cs.html), to ensure visibility. My advice would be to use `lock` instead of `volatile`, because locking is much easier to reason about, compared to the tricky `volatile` keyword. — Theodor Zoulias, Apr 05 '20 at 08:45
@TheodorZoulias I m glad that u looked up the code to see that it's marked volatile indeed, but pls go ahead and see how .Cancel () works. It uses Interlocked class. — Arutyun Enfendzhyan, Apr 05 '20 at 15:16
@ArutyunEnfendzhyan yes, `Cancel` is using `Interlocked` to ensure that only one thread will do the cancellation work. But `IsCancellationRequested` is not using `Interlocked`, and just relies on the volatility of the `m_state` field to ensure visibility. So in OP's case if the `OnDisable` method had to do more work that just changing the `sending` field to `false`, adding the `volatile` keyword would not be enough to ensure that this extra work would be done only once. — Theodor Zoulias, Apr 05 '20 at 15:29

Arutyun Enfendzhyan · Answer 1 · 2020-04-05T08:27:20.573

1

I used to have the same issue with the same task. Fixed it using CancellationTokenSource. Here is how:

 var source = new CancellationTokenSource ();
 Task.Factory.StartNew ( obj => YourMethod ( (CancellationToken) obj ), source.Token, source.Token );

public void YourMethod ( CancellationToken token )
{
    while (!token.IsCancellationRequested)
        ;// insert ur code
}

And to cancel it, store somewhere globally that source variable, then when want to cancel call:

source.Cancel ();

Also u had there this line:

client.Client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.SendTimeout, 5000);

which is completely useless, since UDP by definition is a contactless protocol, so .Connect and .Send don't block the thread. All .Connect does is to set the default IPEndPoint when calling .Send without providing the target IPEndPoint

edited Apr 05 '20 at 08:27

answered Apr 04 '20 at 20:04

Arutyun Enfendzhyan

1,612
1
12
15

Thanks for your answer. I did come across the cancellation token pattern also - but for the sake of edification I would like to know what is preventing the change in this boolean flag from being picked up inside the Task - any idea about that? – Joseph Apr 04 '20 at 20:43
1

Here is an excellent answer:https://stackoverflow.com/questions/29411961/c-sharp-and-thread-safety-of-a-bool. Basically, to be able to retrieve data from another thread, you have to duplicate it using the right context. CancellationToken is simply a storage of the reference to the CancellationTokenSource, which takes care of everything. – Arutyun Enfendzhyan Apr 04 '20 at 20:54
@Joseph R u satisfied with answer? Pls mark it as answered. – Arutyun Enfendzhyan Apr 05 '20 at 08:23
I do appreciate this alternative approach, it is helpful. But please note that the question is "what is wrong" - there has been some useful ifno in the comments that you saw as well, but nobody has put that forward as part of an answer. – Joseph Apr 07 '20 at 01:44

How can I make sure my task is aware of a flag set on the main thread?

1 Answers1