1

I have created a ThreadManager class that handles Threads and its task is to add new threads and clean up the dead threads. However, the threads that are created remain alive and in ThreadState.WaitSleepJoin state. I have checked that the body has successfully finished execution. Any ideas?

    public bool TryAddThread(ThreadStart threadBody, ThreadStartInfo startInfo)
    {
        bool success = false;

        // Validate arguments
        if (threadBody == null || startInfo == null)
        {
            return false;
        }

        if (!Monitor.TryEnter(_lock) || !_allowNewThreads)
        {
            return false;
        }

        try
        {
            Thread newThread = new Thread(threadBody);

            StartThread(newThread, null, startInfo);

            success = true;
        }
        finally
        {
            Monitor.Exit(_lock);
        }

        return success;
    }

    private void StartThread(Thread newThread, object threadParams, ThreadStartInfo startInfo)
    {
        if (newThread == null || startInfo == null)
        {
            return;
        }

        // Apply start info
        newThread.Name = startInfo.Name;
        newThread.SetApartmentState(startInfo.ApartmentState);
        newThread.IsBackground = startInfo.IsBackground;

        if (threadParams == null)
        {
            newThread.Start();
        }
        else
        {
            newThread.Start(threadParams);
        }

        _threads.Add(newThread);

        RemoveDeadThreads();
    }

    public void RemoveDeadThreads()
    {
        _threads.RemoveAll(t => (!t.IsAlive));
    }

Execution in main thread:

    public void InsertAsync(AP p, APr pr)
    {
        ParameterizedThreadStart thread = new ParameterizedThreadStart(InsertPr);
        List<object> parameters = new List<object>();

        // Create new controller. Must be before the thread to avoid cross-thread operation exception.
        PageController controller = new PageController();
        controller.Initialize(siteWebBrowser);

        parameters.Add(controller);
        parameters.Add(p);
        parameters.Add(pr);
        parameters.Add(_session);

        // If the thread cannot start notify listeners
        if (!_threadManager.TryAddThread(thread, parameters, new ThreadStartInfo("InsertAsync", ApartmentState.STA, true)) && ThreadCreationFailed != null)
        {
            _logger.Error("InsertAsync: Could not start thread.");
            ThreadCreationFailed();
        }

    }

    private static void InsertPr(object o)
    {
        try
        {
            _logger.Debug("Thread start - InsertPr");

            List<object> parameters = (List<object>)o;
            PageController controller = (PageController)parameters[0];
            AP p = (AP)parameters[1];
            APr pr = (APr)parameters[2];
            Session session = (Session)parameters[3];

            if (patient == null)
            {
                throw new ArgumentException("Null patient.");
            }

            session.WaitForHistorySynchronizationSuspension();

            if (Program.ShouldAbortBackgroundOperations)
            {
                throw new Exception("Aborting..");
            }

            session.DoingSomeJob = true;



            controller.ClearCurrent();

            controller.GoToHomePage(3, true);

            controller.ClickNew();


            controller.SearchForP(p.Id);


            try
            {
                controller.WaitUntilDivExists(Constants.NewPrContainerDivId);
            }
            catch (Exception)
            {
                _logger.Error("InsertAsync: Error while waiting for div '" + Constants.NewPrContainerDivId + "' to appear.");
                throw;
            }

            if (PrInsertionCompleted != null)
            {
                PrInsertionCompleted();
            }
        }
        catch (Exception ex)
        {
            _logger.ErrorException("InsertAsync", ex);

            if (InsertionFailed != null)
            {
                InsertionFailed(Constants.MessageFailed);
            }
        }
    }
iCantSeeSharp
  • 3,880
  • 4
  • 42
  • 65
  • A couple of things: 1) Why not use the ThreadPool? 2) The thread state depends on what's going on in those threads (do you have any sleeps or wait handles in those delegates?) 3) It looks like there is a possibility that you never release that lock if `_allowNewThreads` is false (you should you the `lock` statement) – SomeWritesReserved Jul 30 '12 at 15:18
  • Without the thread code executing, it's hard to answer this question with much detail. – Peter Ritchie Jul 30 '12 at 15:23
  • I was instructed not to use `ThreadPool` or any other Thread management method. It sounds odd, but those were my instructions. – iCantSeeSharp Jul 30 '12 at 15:27
  • What's the purpose of storing `Thread`s in a list? – SomeWritesReserved Jul 30 '12 at 16:12
  • You have two calls to methods that start with "Wait", seems likely that one of these is putting the thread in a wait state. – Peter Ritchie Jul 30 '12 at 16:13
  • Those methods use WatiN "wait" functions. All the methods return, the code is executed until the last row, bypassing the outer `catch' statement. – iCantSeeSharp Jul 30 '12 at 16:17

2 Answers2

2

You can ask the CLR to automatically abort threads for you when the main startup thread of the program terminates. But that's not automatic, you have to explicitly set the thread's IsBackground property to true. Threadpool threads have that property turned on automatically.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • This is what happens at the moment. Application automatically closes the `Threads`, but I wonder why this happens? Would it be a nice idea to release the lock before actually starting the `Thread`? – iCantSeeSharp Jul 30 '12 at 15:31
  • I don't know what you mean. What does releasing a lock when *starting* a thread have to do with your program terminating? Lock state doesn't matter at all when your process ends. – Hans Passant Jul 30 '12 at 15:36
  • The problem is not closing the threads on application exit. I know they are terminated anyway. The problem is they remain `alive` after their body returns. – iCantSeeSharp Jul 30 '12 at 15:39
  • 1
    "Closing a thread" doesn't mean anything, Thread doesn't have a Close() method nor a Dispose() method. Thus hard to know what you are talking about. Document your question better with the stack trace of such a thread. The specific problem in your code is startInfo.IsBackground. That should always be true. – Hans Passant Jul 30 '12 at 15:43
  • I mean, the threads are killed upon exit. They are all background threads. I don't worry about not being terminated (not closed). I worry about their ThreadState, because I can't find how it is possible in my code and therefore why they remain alive. – iCantSeeSharp Jul 30 '12 at 15:48
  • 1
    I have a serious problem decoding your comments, "how it is possible" to do *what* exactly? Why do you fret about ThreadState when your program is terminating? You should seriously consider chucking this code and use ThreadPool. – Hans Passant Jul 30 '12 at 15:54
  • The Threads are indeed killed upon application exit. But, the `Threads` that are created with `TryAddThread`, remain in `Thread.WaitSleepJoin` even if they successfully finish their job. What I cannot find possible, is the code that causes the threads into that `ThreadState.WaitSleepJoin`, even though I have seen with my eyes the `Monitor.Exit()`. – iCantSeeSharp Jul 30 '12 at 16:01
  • 1
    The Thread class doesn't have a Dispose() method. Which means that the native operating system thread sticks around until the garbage collector runs. GC.Collect() would do it, but calling it just before your program terminates makes no sense. This is btw another issue that the ThreadPool class knows how to deal with. – Hans Passant Jul 30 '12 at 16:07
1

WaitSleepJoin means the thread has blocked itself with a call to lock (Monitor.Enter), a call to Thread.Sleep, or a call to Thread.Join, or some other thread synchronization object.

Maybe if you provide example thread entry point that is causing this thread state, someone can provide a more detailed answer.

Peter Ritchie
  • 35,463
  • 9
  • 80
  • 98
  • I can assure you that the thread body is finished with execution. I have debugged through it and everything worked ok (unfortunately I cannot provide detailed code). I saw the last `}` executing and also the work done by the thread was 100% as expected. The lock was released as well, but the Thread remained in that sleep/wait/join state. – iCantSeeSharp Jul 30 '12 at 15:26
  • Hmmm, I find it hard to believe the system is lying to you... I find it much more likely that the system is correct and that your thread is in fact blocked. Without seeing the code, we can't really tell. – Peter Ritchie Jul 30 '12 at 15:32
  • If I run a quick example of running a thread that simply exits, the state is Stopped and not WaitSleepJoin. – Peter Ritchie Jul 30 '12 at 15:38
  • I have added some extra code. A thread that exits is `Stopped`. Is it `Alive` too? – iCantSeeSharp Jul 30 '12 at 15:49
  • @Souvlaki No, a thread can not be `Stopped` and have `IsAlive` true – Peter Ritchie Jul 30 '12 at 15:56
  • This is what I'm taking about. As you can see in my extra code, I have debugged through it and I saw the last "}" of the thread's body executing. After that, the Thread did not Stop, but remained alive and in `ThreadState.WaitSleepJoin`. – iCantSeeSharp Jul 30 '12 at 16:03
  • No, I can not "see" that... What happens if you simply return from InertPtr and not execute any of the code you posted? Does the thread exit as you'd expect? – Peter Ritchie Jul 30 '12 at 16:12
  • Well, after all it looks like that indeed something in the code of `InsertPr` prevents the Thread from dying. I have skipped most of the lines up to the first event, and next time I tried to execute the same call, the thread was indeed `Stopped`. – iCantSeeSharp Jul 30 '12 at 16:26