CODEWITHSUNDEEP
c#multithreading
silent
silent

Reputation: 16138

It is ok to store a Thread in a static variable?

I want to make sure that I always create only one instance of a Thread so I built this:

private static volatile Thread mdmFetchThread = null;

private static object Locker = new object();

public void myMethod(){

    string someParameter = getParameterDynamically();

    lock(Locker)
    {

    // If an mdmFetchThread is already running, we do not start a new one.
    if(mdmFetchThread != null && mdmFetchThread.ThreadState != ThreadState.Stopped)
    {
       // warn...
    }
    else
    {
        mdmFetchThread = new Thread(() => { doStuff(someParameter); });
        mdmFetchThread.Start();
    }
    }
}

Is this ok to do or what could be possible pitfalls?

//Edit: As requested below a bit context: doStuff() is calling some external system. This call might timeout but I cant specify the timeout. So I call it in mdmFetchThread and do a mdmFetchThread.join(20000) later. To avoid that I call the external system twice, I created the static variable so that I can check if a call is currently ongoing.

Upvotes: 2

Views: 168

Answers (2)

usr
usr

Reputation: 171178

Storing a thread in a static variable is OK (if you need at most one such thread per AppDomain). You can store whatever you want in static storage.

The condition mdmFetchThread.ThreadState != ThreadState.Stopped is racy. You might find it to be false 1 nanosecond before the thread exits. Then you accidentally do nothing. Maintain your own boolean status variable and synchronize properly. Abandon volatile because it is more complicated than necessary.

Consider switching to Task. It is more modern. Less pitfalls.

Consider using a Lazy<Task> to create the singleton behavior you want.

Add error handling. A crash in a background thread terminates the process without notifying the developer of the error.

Upvotes: 3

jt000
jt000

Reputation: 3236

Generally speaking if you are using statics to store state (such as a thread), then you might have a design flaw when attempting to scale out or when trying to manage the lifetime of the object. I usually try to avoid statics whenever possible.

An alternative might be to create a class that only manages a single thread to perform your task as an instance. This class might be responsible for passing data to your Thread or managing the state of it. For example, ensuring it is only run once, stopping the thread gracefully, or handling when the thread completes. If you wanted to scale out, then you'd just create multiple instances of your class each with their own thread that they manage. If you only wanted one, then just pass around a single instance.

If you're looking for ways to make this instance available to your entire application (which is usually the issue people are trying to solve when using static variables), then take a look into patterns like using ServiceContainers and IServiceProvider.

Upvotes: 0

Related Questions