2

Here is my Timer Elapsed Event, I am receiving the System.OutOfMemoryException on the line Thread thread = new Thread(threadStart); I am receiving the error fairly fast (1~5 minutes, randomly), and it does not cause unexpected results in my program. I am just wondering what is causing this error, and I am afraid it may cause unexpected results if it is left unchecked. I have searched on the internet and am comming no where near the number of max threads. readList contains about 46 enteries.

Any help would be appreciated.

private void glob_loopTimer_Elapsed(object sender, ElapsedEventArgs e)
{
    try
    {
        ParameterizedThreadStart threadStart = new ParameterizedThreadStart(readHoldingRegisters);
        foreach (readwriteDataGridRow.Read row in readList)
        {
            Thread thread = new Thread(threadStart);
            thread.IsBackground = true;
            thread.Start(System.Convert.ToInt32(row.Address));
        }
    }
    catch (Exception ex)
    {
        UpdateConsole(new object[] { ex.Message.ToString() + " " + ex.StackTrace.ToString(), Color.Red });
        Thread.CurrentThread.Abort(); // maybe?
    }
}

EDIT: Here is a bit more information. My program is reading registers from a Serial Device using the Modbus RTU protocol. A single register takes less than a tenth of a second to retrieve from readHoldingRegisters

I am open to suggestions on what else to use rather than threads. note: I need to call readHoldingRegisters 40 - 100 times in a single 'pass'. The passes start when the user hits connect and end when he hits disconnect. Timers are not needed, they just offered a simple way for me to maintain the loop with a start and stop button.

EDIT: Solved

    private void glob_loopTimer_Elapsed(object sender, ElapsedEventArgs e)
{
    try
    {
        foreach (readwriteDataGridRow.Read row in readList)
        {
              readHoldingRegisters(row.Address);
        }
    }
    catch (Exception ex)
    {
        UpdateConsole(new object[] { ex.Message.ToString() + " " + ex.StackTrace.ToString(), Color.Red });
    }
}

The additional Threads were the problem and were not needed.

razlebe
  • 7,134
  • 6
  • 42
  • 57
rlemon
  • 17,518
  • 14
  • 92
  • 123
  • What's the interval on your timer? – E.Z. Hart Jul 12 '11 at 13:00
  • 1
    Also how many threads are you creating? Also by this code it doesn't seem like you're cleaning your threads up. – null_pointer Jul 12 '11 at 13:03
  • Yep, appears he is using threads without completely understanding how or when to use threads. – null_pointer Jul 12 '11 at 13:24
  • 1
    Michael, This is why I research and when my implementation fails I turn to development communities to get a better, and more clear grasp of how and when I should be using Threads. Point me in the direction of a `better` tutorial then the ones I have been using. – rlemon Jul 12 '11 at 13:33
  • @relmon Please don't tag your question as solved in the title. Rather, post the answer that solved the problem and mark it as accepted. – razlebe Jul 12 '11 at 14:41

4 Answers4

3

Ughh, do not, ever (well almost ever) abort threads. There are many preferable ways to make a System.Thread stop. Look around SO, you will find plenty of examples on why doing this is a bad idea and alternative approaches.

On with your question: The problem doesn't seem to be the number of rows in readList. It is more likely that your glob_looperTimer_Elapsed event handler is being executed many times and you are basically starting more and more threads.

What is the interval of your glob_loopTimer?

InBetween
  • 32,319
  • 3
  • 50
  • 90
  • The other note on this is he doesn't even need to abort the thread because he is aborting the thread that the timer is running on. If he just catches the exception, call UpdateConsole and and not call abort it would terminate the thread without any problems. – null_pointer Jul 12 '11 at 13:18
  • @Michael: true. Anyhow I wasn't analyzing if it made any sense in this particular scenario, I was just warning that it's use is almost never appropiate. In this case its also useless. – InBetween Jul 12 '11 at 13:20
  • Thnakyou for the information on the Abort, I will update my code accordingly. Also, the timer interval is set to 500 ms, however i receive the error with an interval of 1000 ms and 200 ms. – rlemon Jul 12 '11 at 13:29
  • @rlemon: I think you have to rethink you'r strategy. If the call to `readHoldingRegisters` is expensive (lets say it takes at least 2 seconds) that means that your timer event will fire at least 4 times before the first thread finishes it's task (this means you have 4 x 46 = 184 threads running). You can do the math if the calls are even more expensive. Also, does it make sense to process a whole new event when previous events have not even finished processing yet? Maybe you should consider acquiring a lock in your event handler to ensure that the event driven tasks are scheduled accordingly. – InBetween Jul 12 '11 at 13:35
  • @rlemon: if `readHoldingRegisters` is not that expensive then consider spawning one single thread to read them all instead of one thread per register. – InBetween Jul 12 '11 at 13:54
  • @InBetween Thanks! that worked beautifully. Solution was not to add the additional threads and to just call `readHoldingRegisters` from within the timer/loop I was trying to over complicate things. – rlemon Jul 12 '11 at 14:31
  • @rlemon: Glad you got it working allthough the final solution you implemented is even simpler than what I had in mind. – InBetween Jul 12 '11 at 15:51
2

So how many times is glob_loopTimer_Elapsed called? The name implies that it is run on a periodic timer interval. If so, and if the 46 threads that get created on each invocation do not terminate about as quickly as the timer interval fires, then you could easily be spawning too many threads and running out of memory space as a result. Perhaps you could try logging when each thread starts and when each one finishes to get an idea about how many are in flight at once?

Keep in mind that every thread you allocate will have a certain amount of stack space allocated to it. Depending upon your runtime configuration, this amount of stack space may not be negligible (as in, it may be 1 MB per thread or more) and it may quickly consume your available memory even if you're not close to approaching the theoretical maximum number of threads supported by the OS.

aroth
  • 54,026
  • 20
  • 135
  • 176
2

Besides your problem I'll consider using ThreadPool or the TPL.

When using System.Thread there is no automisn to manage the threads...

Also each Thread allocates some memory which could lead to you problem. The Threadpool and the TPL manage this resources by themselves

see also: -> Thread vs ThreadPool

Reusing threads that have already been created instead of creating new ones (an expensive process)

...

    If you queue 100 thread pool tasks, it will only use as many threads as have already been created to service these requests (say 10

for example). The thread pool will make frequent checks (I believe every 500ms in 3.5 SP1) and if there are queued tasks, it will make one new thread. If your tasks are quick, then the number of new threads will be small and reusing the 10 or so threads for the short tasks will be faster than creating 100 threads up front.

    If your workload consistently has large numbers of thread pool requests coming in, then the thread pool will tune itself to your

workload by creating more threads in the pool by the above process so that there are a larger number of thread available to process requests

    check Here for more in depth info on how the thread pool functions under the hood
Community
  • 1
  • 1
Boas Enkler
  • 12,264
  • 16
  • 69
  • 143
0

I just know Each thread also consumes (by default) around 1 MB of memory.

shenhengbin
  • 4,236
  • 1
  • 24
  • 33