Coffeehouse Thread

20 posts

Question on C# Threads

Back to Forum: Coffeehouse
  • pavone

    So I'm completely new to C# Threading and I'm having some trouble understanding some things. Currently I have a program that does something like this:

    Thread someThread = null;
    
    void foo()
    {
       someThread = new Thread(goo);
       someThread.Start();
    
    }
    
    void goo()
    {
       ...
    }
    
    void bar()
    {
       someThread.Join(); // Null Reference Exception
    }

    I've made that sure that someThread.Start() is being called. I'm guessing it's being set to null after finishing, and it's finishing before I call Join. I'm not too sure how to deal with this, because there may be a chance that the thread is not yet finished, so I don't feel too good about removing the Join since it should be synchronous. Any ideas niners?

    Thanks in advance.  Smiley

  • blowdart

    OK what do you want to do here? Fire off something into a background task, and wait for it to finish before continuing?

    If that's the case you probably don't need to go as low level as the Thread class, there's a few other options, including BackgroundWorker, Task, Plinq, Async, Lions and Tigers oh may!

    Personally I like Task, but then all my use cases are fire and forget. You can also chain tasks, tor example

    var startTask = new Task(() => SomeMethodSomewhere());
    
    var nextTask = startTask.ContinueWith((t) => SomethingElseThatIsRunAfter());
    
    startTask.Start();
    
    

    Task.WaitAll() then gives you a way to wait for completion.

  • ZippyV

    if ( someThread != null && someThread.IsStillRunningOrSomething() )

    someThread.Join();

    EDIT: Obviously, Blowdart's answer is better.

  • evildictait​or

    If you need it to be synchronous, don't use the thread. Threads should be for tasks going on in the background, not for doing critical stuff that you need to wait for.

    As a general rule, if Thread.Join is the answer, you've misunderstodd how to do threading.

    I often use the pattern

    class SomeClass
    {
      Thread bgThread;
      event System.EventHandler SomethingCompleted;
      public SomeClass()
      {
        bgThread = new Thread(() => { bgThreadMain(); });
      }
      public void StartBackgroundWork()
      {
        bgThread.Start();
      }
      private void bgThreadMain()
      {
        doSomethingThatMightTakeAWhile();
        if(SomethingCompleted != null) SomethingCompleted(this, EventArgs.Empty);
      }
      private void doSomethingThatMightTakeAWhile()
      {
        Process.Start("ping", "127.0.0.1");
      }
    }

    You then use it like:

    void Foo()
    {
      volatile bool keepDrawingAnimation = false;
      var x = new SomeClass();
      x.SomethingCompleted += new EventHandler( (o,e) => { keepDrawingAnimation = false; });
      x.Start();
      Console.WriteLine("Please wait...");
      while(keepDrawingAnimation)
      {
        Console.Write("."); Thread,Sleep(100);
      }
      Console.WriteLine("Done");
    }

  • blowdart

    , evildictait​or wrote

    If you need it to be synchronous, don't use the thread. Threads should be for tasks going on in the background, not for doing critical stuff that you need to wait for.

    Oh not true sir. If I have 3 running tasks that are isolated from each other, but something needs the results of all 3 then that's a perfect use for threading.

     

  • pavone

     Thanks guys, it needs to be synchronous with respect to that one function, it should run in parallel with other threads before then. I wasn't too excited on using simply checking for null, because then I'm not sure if the thread ever ran at all.

    I'll look into Tasks and see if it solves my problem.  

    Update 1

    Not sure if tasks is what I need since this is an Event driven app. When a user does something I then try to Join the thread that should have been started a while ago and will most likely be finished. 

    Update 2

    I feel a little more background would be of use. I have an app that currently loads information from a database when user selects an option from a drop down menu, there are few options, so I though I could pre-fetch the most used ones so there's no delay. 

  • evildictait​or

    , blowdart wrote

    *snip*

    Oh not true sir. If I have 3 running tasks that are isolated from each other, but something needs the results of all 3 then that's a perfect use for threading.

    Perhaps I should have been more clear - I meant Thread the type, not Threading the concept.

    System.Threading.Threads should be for tasks going on in the background.

    If you're dealing with concurrent actions that you want to use in parallel, use something like Future<T> (or possibly BackgroundWorker).

    Future<int> a = new Future<T>( () => Gen1 );
    Future<int> b = new Future<T>( () => Gen2 );
    Future<int> c = new Future<T>( () => Gen3 );
    int d = Gen4();

    return d + a.Value + b.Value + c.Value;

     

    System.Threading.Thread shouldn't be used for anything other than a long-running parallel thread of execution (e.g. parsing a file in the background or handling incoming/outgoing network and pipe connections) - and in general you shouldn't need to Join with it to block the UI, you'll want to expose an event instead to let the UI remain smooth whilst doing the processing. System.Threading.Thread has a punishingly high startup cost. Don't use them for short running tasks or stuff that you're going to wait on. (Task and BackgroundWorker amortize the cost by using a threadpool, which is much better).

  • spivonious

    @pavone: Why not check out the BackgroundWorker class? It's designed to fire off some task while keeping the UI thread free.

  • blowdart

    , pavone wrote

     Thanks guys, it needs to be synchronous with respect to that one function, it should run in parallel with other threads before then. I wasn't too excited on using simply checking for null, because then I'm not sure if the thread ever ran at all.

    I'll look into Tasks and see if it solves my problem.  

    Update 1

    Not sure if tasks is what I need since this is an Event driven app. When a user does something I then try to Join the thread that should have been started a while ago and will most likely be finished. 

    Update 2

    I feel a little more background would be of use. I have an app that currently loads information from a database when user selects an option from a drop down menu, there are few options, so I though I could pre-fetch the most used ones so there's no delay. 

    Ah ok. Well, sticking with Task you chould in your class have your

    Task databaseLoader = new Task blahblahblah

    and start it running when you app starts.

    Then inside your event you can check the databaseLoader.IsCompleted(), if it's not (and it's not faulted) then Task.WaitAll(databaseLoader);

  • pavone

    Sounds good blowdart, except I'm limited to .NET Framework 3.5 and Task does not seem to be available in my .NET references... 

    I'm currently looking into BackgroundWorker, I've been reading a bit on it and it sounds pretty good. I wasn't too worried about Threads using up too many resources because I spawn very very few of them anyways, but it doesn't hurt to recycle. 

    You guys have been of great help though. 

  • magicalclick

    the dumb way is to have a static volatile int. You increment it by one before you start the thread. And in the thread, you decrement it by one at the end. And in your main thread, just brainlessly waiting the int to become 0.

    Yupe, quite dumb, but, if you are not doing anything serious, the more dumb the better. Big Smile

    Leaving WM on 5/2018 if no apps, no dedicated billboards where I drive, no Store name.
    Last modified
  • kettch

    @magicalclick: There are too many applications that start out as nothing serious, and then become serious, but still have the dumb code.

  • Sven Groot

    @magicalclick: And how do you wait for the int to become zero? Polling? Not to mention that addition/subtraction is not thread safe unless you're using interlocked operation. This method of doing it is not just dumb, but stupid.

    If you want to go that route you can use WaitHandles, or a Semaphore, or in .Net 4.0 the CountdownEvent (which is a thread-safe way of doing what you described).

    There's a simple rule with multi-threading: if you think you've found a simple way to do something, it's nearly always wrong (unless that simple way is using a class someone else wrote that encapsulates the hard stuff, like Task or BackgroundWorker).

  • evildictait​or

    , magicalclick wrote

    the dumb way is to have a static volatile int. You increment it by one before you start the thread. And in the thread, you decrement it by one at the end. And in your main thread, just brainlessly waiting the int to become 0.

    Yupe, quite dumb, but, if you are not doing anything serious, the more dumb the better. Big Smile

    It's harder to do that right (i.e. in a thread-safe way) than to just use a BackgroundWorker. Hence I would say that the BackgroundWorker is a better choice for beginners.

  • AndyC

    , Sven Groot wrote

    There's a simple rule with multi-threading: if you think you've found a simple way to do something, it's nearly always wrong (unless that simple way is using a class someone else wrote that encapsulates the hard stuff, like Task or BackgroundWorker).

    +1

  • BitFlipper

    The OP's original question has not been answered. I think there is something going on there because if you first call foo() and then later bar(), there is no way that there could be a null ref exception in bar(). someThread will not be set to null automatically. We are not seeing all of the code so we can't answer that question.

    Maybe someThread is really a static in the actual code and it is being set to null in the constructor? That would cause one instance to set it to null after another instance is using it. Just guessing since we don't have all of the code.

  • evildictait​or

    , BitFlipper wrote

    We are not seeing all of the code so we can't answer that question.

    And therein lies the problem.

    If someone asks a question about "why does my code not work" without providing at least an example that demonstrates the problem, the only option we have is to try psychic debugging to solve it.

    If the OP wants it fixed, s/he needs to post an example that demonstrates the problem s/he's trying to solve.

  • pavone

     It's rather interesting, I tried to replicate the error in a console application and it worked just fine, no null reference, even though the logic is the same. Initially I thought it might have to do with scope. The code that's giving me the error is an ASP.NET webpage and the Thread logic is exactly as described in the first post.

    In global scope, I have defined  a thread and a static list (which is modified by such thread) and set thread to null and list to new List<string>().

    I did a workaround by checking that the list has been changed, if not the it goes back to the old way of fetching after user request. Still, it intrigues me why that error is being thrown and I seem unable to replicate, it'll probably be something very simple I'm overlooking.

    All the relevant code:

    protected void ddlRelTypeSelected(object sender, EventArgs e)
            {
                lblRelType.Visible = true;
    
                string strALPHA ="";
               
                if (ddlPlatform.Text == "IVWEX")
                {
                    if (ddlType.Text == "QA.Builds" || ddlType.Text == "QA.Releases")
                    {
                        // ...
                    }
                    else
                    {
                        // ...
                    }
                }
                else if (ddlPlatform.Text == "ALPHA")
                {
                    if (ddlType.Text == "QA.Candidates")
                    {
                        // ...
                    }
                    else if (ddlType.Text == "SCM.QA_Images")
                    {
                        // ...
                    }
                    else    // QA.Releases
                    {
                        strALPHA = ConfigurationManager.AppSettings["RelTypeAlpha"].ToString();
                        string[] arrALPHA = strALPHA.Split(chsep);
                        ddlRelType.DataSource = arrALPHA;
    
                        // spawn osThread and gameThread
                        osThread = new Thread(() => getReleases("OS", alist));   
                        osThread.Start();
                        gamesThread = new Thread(() => getReleases("GAMES", blist));
                        gamesThread.Start();
                    }
                    ddlRelType.DataBind();
                    ddlRelType.Visible = true;
                }     
            }
    
            private void getReleases(string SType, List<string> list)
            {
                OleDbConnection conn = openDBConnection(SCMSoftwareReleases);
                string SqlQuery = " SELECT Release_Label as ReleaseLabel, Release_Type as ReleaseType "
                          + " FROM TrackReleases "
                          + " WHERE Platform = 'ALPHA' "
                          + " AND Software_Type = '" + SType + "' "
                          + " AND Release_Type <> 'Branch' "
                          + " ORDER by 1 asc";
                OleDbDataAdapter da = new OleDbDataAdapter(SqlQuery, conn);
                DataSet ds = new DataSet();
                da.Fill(ds);
                DataRowCollection drc = ds.Tables["Table"].Rows;    // Table is default name
                list.Clear();
                
                for (int i = 0; i < drc.Count; ++i)
                    list.Add(drc[i][0].ToString() + "|" + drc[i][1].ToString());
                if (conn != null)
                    conn.Close();
            }
    
            private void fetchReleases(ref string[] arr, string SType)
            {
                osThread.Join();    // null reference exception
                
                // ...
            }

    The webpage is a bunch of dropdown menus that leads to the sql query. The threads are without a doubt being spawned before fetchReleases is called because fetchReleases can only be called from a UI element that only appears after the call to ddlRelTypeSelected event handler.

    Actually stepping through the debugger shows me that threads are finished before going to fetchReleases.
    Now I'm thinking it's due to being a web app, the stateless nature of web apps has gotten me again?!

Comments closed

Comments have been closed since this content was published more than 30 days ago, but if you'd like to continue the conversation, please create a new thread in our Forums, or Contact Us and let us know.