Click here to Skip to main content
Rate this: bad
good
Please Sign up or sign in to vote.
See more: C# Threading
I have a network monitor program which continually monitors a list of ip addresses on the network and writes the log to a log file. The program works all right but after a few days of continuous running the UI hangs up and the task manager starts showing that my process is making too much CPU usage, which is quite unnatural. Any help to make it better will be very welcome.
 
Thanks in advance.
 
P.S. - I am including the code for a quick review.
 
MainForm.cs
using System;
using System.IO;
using System.Net;
using System.Xml;
using System.Threading;
using System.Diagnostics;
using System.Net.NetworkInformation;
using System.Collections.Generic;
using System.Windows.Forms;
 
namespace NetworkMonitor
{
    public partial class MainForm : Form
    {
        private List<NetworkLocation> LocationList;
        private delegate void nlss(NetworkLocation nl);
        private Thread PingSender;
        private LogWriter writer;
 
        public MainForm()
        {
            InitializeComponent();
            LocationList=new List<NetworkLocation>();
            PingSender = new Thread(this.CheckIpAsync);
            writer = new LogWriter();
        }
 
        public bool AddNetworkLocation(string name,string address)
        {
            NetworkLocation temp;
 
            if (name.Length == 0)
            {
                MessageBox.Show("Enter a name","Network Monitor",MessageBoxButtons.OK);
                return false;
            }
            else if (!ValidIp(address))
            {
                MessageBox.Show("Enter a valid ip/hostname", "Network Monitor", MessageBoxButtons.OK);
                return false;
            }
 
            temp=new NetworkLocation(name,address);
            LocationList.Add(temp);
            return true;
        }
 
        private bool ValidIp(string address)
        {
            string[] parts = address.Split(new char[] { '.' }, StringSplitOptions.None);
 
            if (parts.Length != 4)
                return false;
            else if ( parts[0].Length == 0 || parts[1].Length == 0 || parts[2].Length == 0 || parts[3].Length == 0)
                return false;
            else
            {
                try
                {
                    int a, b, c, d;
 
                    a = int.Parse(parts[0]);
                    b = int.Parse(parts[1]);
                    c = int.Parse(parts[2]);
                    d = int.Parse(parts[3]);
 
                    if ((a < 0 || a > 255) || (b < 0 || b > 255) || (c < 0 || c > 255) || (d < 0 || d > 255))
                        return false;
                    else
                        return true;
                }
                catch (Exception)
                {
                    return false;
                }
            }
        }
 
        public void CheckIpAsync()
        {
            try
            {       
                while (true)
                {
                    NetworkLocation[] all_locations = new NetworkLocation[LocationList.Count];
 
                    LocationList.CopyTo(all_locations);
                    foreach (NetworkLocation nl in all_locations)
                    {
                        this.BeginInvoke(new nlss(this.NetworkLocationSetStatus), new Object[] { nl });
                    }
                    Thread.Sleep(1000);
                }
            }
            catch (Exception)
            {
                writer.WriteLine("IP checking stopped at " + DateTime.Now);
                writer.WriteLine("-------------------------------------------------------------------");
                writer.Stop();
            }            
        }
 
        public void NetworkLocationSetStatus(NetworkLocation nl)
        {            
            try
            {
                Ping ipchecker = new Ping();
                PingReply reply = ipchecker.Send(nl.Address, 1000);
 
                if (reply.Status == IPStatus.Success && nl.IsAlive == false)
                {
                    writer.WriteLine(nl.Name + "[" + nl.Address + "] went alive at " + DateTime.Now);
                    nl.IsAlive = true;
                }
                else if (reply.Status != IPStatus.Success && nl.IsAlive == true)
                {
                    writer.WriteLine(nl.Name + "[" + nl.Address + "] went dead at " + DateTime.Now);
                    nl.IsAlive = false;
                }
            }
            catch (Exception)
            {
                writer.WriteLine(nl.Name + "[" + nl.Address + "] went dead at " + DateTime.Now);
            }
        }
 
        private void AddLocationButton_Click(object sender, EventArgs e)
        {
            AddLocation f = new AddLocation(this);
            f.ShowDialog();
        }
 
        private void AboutButton_Click(object sender, EventArgs e)
        {
            NetworkMonitorAboutBox f = new NetworkMonitorAboutBox();
            f.ShowDialog();
        }
 
        private void Watchmantimer_Tick(object sender, EventArgs e)
        {
            int index = -1;
 
            try
            {
                index = LocationListView.SelectedIndices[0];
            }
            catch (Exception)
            {
            }
            finally
            {
                LocationListView.Items.Clear();
 
                foreach (NetworkLocation l in LocationList)
                {
                    ListViewItem li = LocationListView.Items.Add(l.Name);
                    li.SubItems.Add(l.Address);
                    li.ImageIndex = l.IsAlive ? 0 : 1;
                }
 
                if (index >= LocationListView.Items.Count)
                    index = -1;
 
                if (index != -1)
                    LocationListView.Items[index].Selected = true;
            }
        }
 
        private void MainForm_FormClosing(object sender, FormClosingEventArgs e)
        {
            DialogResult rs = MessageBox.Show("Click Yes to close application \nClick No to minimize\n& Click Cancel to close this dialog", "Network Monitor", MessageBoxButtons.YesNoCancel);
 
            if (rs == DialogResult.Yes)
            {
                if (PingSender.IsAlive)
                {
                    PingSender.Abort();
                }
 
                try
                {
                    File.Delete("NetworkMonitor.xml");
 
                    XmlTextWriter xmlWriter = new XmlTextWriter("NetworkMonitor.xml", System.Text.Encoding.UTF8);
                    //creating XmlTestWriter, and passing file name and encoding type as argument
                    xmlWriter.Formatting = Formatting.Indented;
                    //setting XmlWriter formating to be indented
                    xmlWriter.WriteProcessingInstruction("xml", "version='1.0' encoding='UTF-8'");
                    //writing version and encoding type of XML in file.
                    xmlWriter.WriteStartElement("NetworkLocationList");
                    //writing first element
                    xmlWriter.Close();
 
                    XmlDocument doc = new XmlDocument();                    //add to file
                    doc.Load("NetworkMonitor.xml");
                    foreach (NetworkLocation nl in LocationList)
                    {
                        XmlElement newlocation = doc.CreateElement("NetworkLocation");
                        XmlElement newname = doc.CreateElement("Name");
                        XmlElement newaddress = doc.CreateElement("Address");
 
                        newname.InnerText = nl.Name;
                        newaddress.InnerText = nl.Address;
                        newlocation.AppendChild(newname);
                        newlocation.AppendChild(newaddress);
                        doc.DocumentElement.AppendChild(newlocation);
                    }
                    XmlTextWriter tr = new XmlTextWriter("NetworkMonitor.xml", null);
                    tr.Formatting = Formatting.Indented;
                    doc.WriteContentTo(tr);
                    tr.Close();
                }
                catch (Exception)
                {
                    MessageBox.Show("Could not save entries while closing form", "Network Monitor", MessageBoxButtons.OK);
                }
            }
            else if (rs == DialogResult.No)
            {
                this.WindowState = FormWindowState.Minimized;
                this.MainForm_Resize(this, null);
            }
            else
                e.Cancel = true;
        }
 
        private void ApplicationNotifyIcon_MouseDoubleClick(object sender, MouseEventArgs e)
        {
            this.Show();
            this.WindowState = FormWindowState.Normal;
        }
 
        private void MainForm_Resize(object sender, EventArgs e)
        {
            if (this.WindowState == FormWindowState.Minimized)
            {
                ApplicationNotifyIcon.Visible = true;
                ApplicationNotifyIcon.ShowBalloonTip(200);
                this.Hide();
            }
            else if (FormWindowState.Normal == this.WindowState)
            {
                ApplicationNotifyIcon.Visible = false;
            }
        }
 
        private void ShowLogButton_Click(object sender, EventArgs e)
        {
            if (File.Exists(Environment.CurrentDirectory + "\\" + DateTime.Now.ToString("ddMMyyyy") + ".log"))
            {
                Process showlog = new Process();
                showlog.StartInfo.FileName = "notepad.exe";
                showlog.StartInfo.Arguments = Environment.CurrentDirectory + "\\" + DateTime.Now.ToString("ddMMyyyy") + ".log";
                showlog.Start();
            }
            else
                MessageBox.Show("No Log created for today. Try again later.", "Network Monitor", MessageBoxButtons.OK);
 
        }
 
        private void RemoveLocationButton_Click(object sender, EventArgs e)
        {
            try
            {
                int index = LocationListView.SelectedIndices[0];
 
                LocationList.RemoveAt(index);
            }
            catch (Exception)
            {
                MessageBox.Show("Select a location from the list and then click Remove", "Network Monitor", MessageBoxButtons.OK);
            }
        }
 
        private void MainForm_Load(object sender, EventArgs e)
        {
            writer.WriteLine("Application started at " + DateTime.Now);
 
            XmlDocument doc = new XmlDocument();
            doc.Load("NetworkMonitor.xml");
            XmlNodeList nodelst = doc.GetElementsByTagName("NetworkLocation");
            foreach (XmlNode nd in nodelst)
            {
                this.AddNetworkLocation(nd.FirstChild.InnerText, nd.LastChild.InnerText);
            }
 
            PingSender.Priority = ThreadPriority.BelowNormal;
            PingSender.Start();
        }
 
    }
}
 
 
LogWriter.cs
using System;
using System.IO;
using System.Threading;
using System.Collections.Generic;
 
namespace NetworkMonitor
{
    public class LogWriter
    {
        private List<String> _messagelist;
        private Thread _writer;
 
        public LogWriter()
        {
            _messagelist = new List<String>();
            _writer = new Thread(WriteAsync);
 
            _writer.Start();
        }
 
        public void WriteLine(String message)
        {
            if (message.ToString().Length > 0)
            {
                _messagelist.Add(message);
            }
        }
 
        public void Stop()
        {
            _writer.Abort();
        }
 
        private void WriteAsync()
        {
            try
            {
 
                while (true)
                {
                    try
                    {
 
                        String filename = Environment.CurrentDirectory + "\\" + DateTime.Now.ToString("ddMMyyyy") + ".log";
                        if (!File.Exists(filename))
                            File.Create(filename);
 
                        using (StreamWriter writer = new StreamWriter(filename,true))
                        {
                            String[] messagelistcopy = _messagelist.ToArray();
                            _messagelist.Clear();
 
                            foreach (String msg in messagelistcopy)
                            {
                                writer.WriteLine(msg);
                            }
                            writer.Flush();
                        }
                    }
                    catch (Exception)
                    {
                        //do nothing
                    }
 
                }
            }
            catch (Exception)
            {
                //do nothing
            }
        }
    }
}
Posted 21-Jan-13 2:23am
Comments
Fred Flams at 21-Jan-13 10:09am
   
Hi there,
 
After reviewing your code I have two questions:
1) When do you clear you _messageList buffer ?
2) Why didn't you limit the amount of disk space that your log file can use ? (I strongly suspect, that your log file is becoming too large for the framework to handle)
3) Why did you write your own logger, when you could have used a framework such as log4net that is freely downloadable and does all the logging staff for you, including log file rolling (i.e. log file splitting, and archiving), it can also log to other medium than a simple file.
 
Hope this remarks will help you solve your problem.
Fred Flams at 21-Jan-13 10:11am
   
Sorry did not see that your log file is constantly overwritten, there's only left your _messageList buffer that don't get emptied when handled and that list can become pretty huge from what I understand.
Member 7715229 at 21-Jan-13 23:29pm
   
1)_messageList buffer is cleared by the WriteASync() method which runs on a thread.
2) i didn't limit log file usage beacuse i day's file usage turns out <30kb , which i think isn't much.
3) i didn't know that, thanks for the suggestion. i'll surely look it up (who wants to re-invent the wheel when its already there ;-)!!!!!)
jibesh at 21-Jan-13 18:25pm
   
I can see some serious synchronization issue with this design. your logger resource can be used my multiple threads at a time and can lead to serious data handling issue also you are trying to access '_messagelist' variable inside your 'WriteAsync' thread procedure without any protection. by having a proper locking statement your problem may get resolved.
Member 7715229 at 21-Jan-13 23:32pm
   
can u suggest something for synchronization. i guessed i might have a problem, so i copied the whole contents into another array and then cleared the original one.
 
P.S. - the logger actually runs pretty well in the background even if the UI hangs up.
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 3

You are completely missing any thread synchronization.
 
Make yourself a picture how many threads are involved and which instances are shared between the threads.
 
E.g. LogWrite.WriteLine is called over multiple threads leading to unprotected _messagelist access. This allone is just a matter of time until bad things happen.
_messagelist is also internally unprotectedly modified by the internal thread concurently to the WriteLine calls.
 
Similar issues with the rest of the application.
 
If playing with threads, you must have a clear threading schema (how many threads are running, what data do they share) and a clear locking schema (what data must be protected from concurrent modification). E.g. draw sequence diagrams with all threads and all shared objects.
 
Cheers
Andi
  Permalink  
Comments
Member 7715229 at 21-Jan-13 23:46pm
   
can u give me a decent example??? u got a clear idea of what i want to do. can u suggest me a good locking solution????
Thanks in advance
Andreas Gieriet at 22-Jan-13 1:46am
   
E.g. work this through.
Multi-threading allows to start concurrent control flows that may share data. To avoid multiple threads to modify the shared data (e.g. your _messagelist) at the very same time (e.g. removing data while adding others) you must provide means to put such concurrent access in sequence. I.e. you protect every access to the shared data e.g. by a c# lock block. For that, you as the designer and implementer of the application must know what is supposed to be shared and what is not exposed to concurrent access. That's your task - noone can take that burden from you, it's your design.
If you work with threads you must know the basics of threading. Take any book or any tutorial on multi-threading and learn it. It's like learning driving a car. There it's obvious that a novice cannot start the car and drive on. You need to know the basics of traffic control, diving on the proper side of the road, stop where you are supposed to stop, learn how to start/stop the car, change gears, share the road with others, ...
Spend that time to learn multi-threading basics (especially what protecting access to shared data means).
Andi
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 4

using System;
using System.IO;
using System.Net;
using System.Xml;
using System.Threading;
using System.Diagnostics;
using System.Net.NetworkInformation;
using System.Collections.Generic;
using System.Windows.Forms;
using CodeProjectSamples;
 
namespace NetworkMonitor
{
    public partial class MainForm : Form
    {
        private List<NetworkLocation> LocationList;
        private ListView LocationListView;
    
        private delegate void nlss(NetworkLocation nl);
        private Thread PingSender;
 
        LogWriter writer;
        public MainForm()
        {
            writer = new LogWriter();
            InitializeComponent();
            LocationList = new List<NetworkLocation>();
            PingSender = new Thread(this.CheckIpAsync);
            AddNetworkLocation("localhost", "127.0.0.1");
            PingSender.Start();
        }
 
        public bool AddNetworkLocation(string name, string address)
        {
            NetworkLocation temp;
 
            if (name.Length == 0)
            {
                MessageBox.Show("Enter a name", "Network Monitor", MessageBoxButtons.OK);
                return false;
            }
            else if (!ValidIp(address))
            {
                MessageBox.Show("Enter a valid ip/hostname", "Network Monitor", MessageBoxButtons.OK);
                return false;
            }
 
            temp = new NetworkLocation(name, address);
            LocationList.Add(temp);
            return true;
        }
 
        private bool ValidIp(string address)
        {
            string[] parts = address.Split(new char[] { '.' }, StringSplitOptions.None);
 
            if (parts.Length != 4)
                return false;
            else if (parts[0].Length == 0 || parts[1].Length == 0 || parts[2].Length == 0 || parts[3].Length == 0)
                return false;
            else
            {
                try
                {
                    int a, b, c, d;
 
                    a = int.Parse(parts[0]);
                    b = int.Parse(parts[1]);
                    c = int.Parse(parts[2]);
                    d = int.Parse(parts[3]);
 
                    if ((a < 0 || a > 255) || (b < 0 || b > 255) || (c < 0 || c > 255) || (d < 0 || d > 255))
                        return false;
                    else
                        return true;
                }
                catch (Exception)
                {
                    return false;
                }
            }
        }
 
        public void CheckIpAsync()
        {
            try
            {
                while (true)
                {
                 
                    NetworkLocation[] all_locations = new NetworkLocation[LocationList.Count];
 
                    LocationList.CopyTo(all_locations);
                    foreach (NetworkLocation nl in all_locations)
                    {
                        this.BeginInvoke(new nlss(this.NetworkLocationSetStatus), new Object[] { nl });
                    }
                    Thread.Sleep(1000);
                }
            }
            catch (Exception)
            {
                
            }
        }
 
        public void NetworkLocationSetStatus(NetworkLocation nl)
        {
            try
            {
                Ping ipchecker = new Ping();
                PingReply reply = ipchecker.Send(nl.Address, 1000);
 
                if (reply.Status == IPStatus.Success && nl.IsAlive == false)
                {
                    MessageBox.Show(nl.Name + "[" + nl.Address + "] went alive at " + DateTime.Now);
                    writer.WriteLine(nl.Name + "[" + nl.Address + "] went alive at " + DateTime.Now);
                    
                    nl.IsAlive = true;
                }
                else if (reply.Status != IPStatus.Success && nl.IsAlive == true)
                {
                    writer.WriteLine(nl.Name + "[" + nl.Address + "] went dead at " + DateTime.Now);
                    nl.IsAlive = false;
                }
            }
            catch (Exception)
            {
                writer.WriteLine(nl.Name + "[" + nl.Address + "] went dead at " + DateTime.Now);
            }
        }
 
        private void AddLocationButton_Click(object sender, EventArgs e)
        {
           // AddLocation f = new AddLocation(this);
            //f.ShowDialog();
        }
 
        private void AboutButton_Click(object sender, EventArgs e)
        {
            //NetworkMonitorAboutBox f = new NetworkMonitorAboutBox();
            //f.ShowDialog();
        }
 
        private void Watchmantimer_Tick(object sender, EventArgs e)
        {
            int index = -1;
 
            try
            {
                index = LocationListView.SelectedIndices[0];
            }
            catch (Exception)
            {
            }
            finally
            {
                LocationListView.Items.Clear();
 
                foreach (NetworkLocation l in LocationList)
                {
                    ListViewItem li = LocationListView.Items.Add(l.Name);
                    li.SubItems.Add(l.Address);
                    li.ImageIndex = l.IsAlive ? 0 : 1;
                }
 
                if (index >= LocationListView.Items.Count)
                    index = -1;
 
                if (index != -1)
                    LocationListView.Items[index].Selected = true;
            }
        }
 
        private void MainForm_FormClosing(object sender, FormClosingEventArgs e)
        {
            DialogResult rs = MessageBox.Show("Click Yes to close application \nClick No to minimize\n& Click Cancel to close this dialog", "Network Monitor", MessageBoxButtons.YesNoCancel);
 
            if (rs == DialogResult.Yes)
            {
                if (PingSender.IsAlive)
                {
                    PingSender.Abort();
                }
 
                try
                {
                    File.Delete("NetworkMonitor.xml");
 
                    XmlTextWriter xmlWriter = new XmlTextWriter("NetworkMonitor.xml", System.Text.Encoding.UTF8);
                    //creating XmlTestWriter, and passing file name and encoding type as argument
                    xmlWriter.Formatting = Formatting.Indented;
                    //setting XmlWriter formating to be indented
                    xmlWriter.WriteProcessingInstruction("xml", "version='1.0' encoding='UTF-8'");
                    //writing version and encoding type of XML in file.
                    xmlWriter.WriteStartElement("NetworkLocationList");
                    //writing first element
                    xmlWriter.Close();
 
                    XmlDocument doc = new XmlDocument();                    //add to file
                    doc.Load("NetworkMonitor.xml");
                    foreach (NetworkLocation nl in LocationList)
                    {
                        XmlElement newlocation = doc.CreateElement("NetworkLocation");
                        XmlElement newname = doc.CreateElement("Name");
                        XmlElement newaddress = doc.CreateElement("Address");
 
                        newname.InnerText = nl.Name;
                        newaddress.InnerText = nl.Address;
                        newlocation.AppendChild(newname);
                        newlocation.AppendChild(newaddress);
                        doc.DocumentElement.AppendChild(newlocation);
                    }
                    XmlTextWriter tr = new XmlTextWriter("NetworkMonitor.xml", null);
                    tr.Formatting = Formatting.Indented;
                    doc.WriteContentTo(tr);
                    tr.Close();
                }
                catch (Exception)
                {
                    MessageBox.Show("Could not save entries while closing form", "Network Monitor", MessageBoxButtons.OK);
                }
            }
            else if (rs == DialogResult.No)
            {
                this.WindowState = FormWindowState.Minimized;
                this.MainForm_Resize(this, null);
            }
            else
                e.Cancel = true;
        }
 
        private void ApplicationNotifyIcon_MouseDoubleClick(object sender, MouseEventArgs e)
        {
            this.Show();
            this.WindowState = FormWindowState.Normal;
        }
 
        private void MainForm_Resize(object sender, EventArgs e)
        {
            if (this.WindowState == FormWindowState.Minimized)
            {
                //ApplicationNotifyIcon.Visible = true;
                //ApplicationNotifyIcon.ShowBalloonTip(200);
                //this.Hide();
            }
            else if (FormWindowState.Normal == this.WindowState)
            {
                //ApplicationNotifyIcon.Visible = false;
            }
        }
 
        private void ShowLogButton_Click(object sender, EventArgs e)
        {
            if (File.Exists(Environment.CurrentDirectory + "\\" + DateTime.Now.ToString("ddMMyyyy") + ".log"))
            {
                Process showlog = new Process();
                showlog.StartInfo.FileName = "notepad.exe";
                showlog.StartInfo.Arguments = Environment.CurrentDirectory + "\\" + DateTime.Now.ToString("ddMMyyyy") + ".log";
                showlog.Start();
            }
            else
                MessageBox.Show("No Log created for today. Try again later.", "Network Monitor", MessageBoxButtons.OK);
 
        }
 
        private void RemoveLocationButton_Click(object sender, EventArgs e)
        {
            try
            {
                int index = LocationListView.SelectedIndices[0];
 
                LocationList.RemoveAt(index);
            }
            catch (Exception)
            {
                MessageBox.Show("Select a location from the list and then click Remove", "Network Monitor", MessageBoxButtons.OK);
            }
        }
 
        private void MainForm_Load(object sender, EventArgs e)
        {
            writer.WriteLine("Application started at " + DateTime.Now);
 
            XmlDocument doc = new XmlDocument();
            doc.Load("NetworkMonitor.xml");
            XmlNodeList nodelst = doc.GetElementsByTagName("NetworkLocation");
            foreach (XmlNode nd in nodelst)
            {
                this.AddNetworkLocation(nd.FirstChild.InnerText, nd.LastChild.InnerText);
            }
 
            PingSender.Priority = ThreadPriority.BelowNormal;
            PingSender.Start();
        }
 
        private void InitializeComponent()
        {
            this.LocationListView = new System.Windows.Forms.ListView();
            this.SuspendLayout();
            // 
            // LocationListView
            // 
            this.LocationListView.Location = new System.Drawing.Point(32, 27);
            this.LocationListView.Name = "LocationListView";
            this.LocationListView.Size = new System.Drawing.Size(504, 282);
            this.LocationListView.TabIndex = 0;
            this.LocationListView.UseCompatibleStateImageBehavior = false;
            // 
            // MainForm
            // 
            this.ClientSize = new System.Drawing.Size(669, 365);
            this.Controls.Add(this.LocationListView);
            this.Name = "MainForm";
            this.ResumeLayout(false);
 
        }
 
    }
}
 
The above is his main form and here is NetworkLocation class
 
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
 
namespace CodeProjectSamples
{
  public class NetworkLocation
    {
        public string Address;
        public string Name;
        public bool IsAlive;
 
        public NetworkLocation(string name, string address)
        {
            this.Address = address;
            this.Name = name;
        }
    }
}
 

Now if you execute the code ( It crashed my system twice. So be careful), you will see that the MessageBox I have placed inside CheckIpAsync() never gets called. As against if you comment while(true), expectantly once the MessageBox will appear. Why?
 
OP assumes that by invoking CheckIpAsync from an Asynchronous callback aspect fixes everything for him. No. In LogWriter class there is a shared variable
 _messagelist
.
 
So every time you call WriteLine() you are trying to write in _messagelist. Result? Two asynchronous instances at some instance will write to access the _messagelist memory.
 
Now do a simple test:
 
Change the foreach statement with
 
this.Invoke((MethodInvoker)delegate
                        {
                            this.BeginInvoke(new nlss(this.NetworkLocationSetStatus), new Object[] { nl });
                        });

 
You will see many instances of MessageBox. So either you need to use a BackgroundWorker and modify your while with that or take a timer and call CheckIpAsync from timer tick ( no while remember). In either cases protect _messagelist by proper locking ( eg. above code).
 
In my discussion in comments I took an example of Label because UI elements are better way of understanding the flow of your execution. calling BeginInvoke from a loop with a shared variable is as good as calling it serially due to lock problem.
  Permalink  
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 5

the problem lay elsewhere. the ui became unresponsive only when there was a unreachable node in the addresslist. this happened beacause my code was somewhat like this
 
public void CheckIpAsync()
        {
            try
            {       
                while (true)
                {
                    NetworkLocation[] all_locations = new NetworkLocation[LocationList.Count];
 
                    LocationList.CopyTo(all_locations);
                    foreach (NetworkLocation nl in all_locations)
                    {
                        this.BeginInvoke(new nlss(this.NetworkLocationSetStatus), new Object[] { nl });
                    }
                    Thread.Sleep(1000);
                }
            }
            catch (Exception)
            {
                writer.WriteLine("IP checking stopped at " + DateTime.Now);
                writer.WriteLine("-------------------------------------------------------------------");
                writer.Stop();
            }            
        }
 
        public void NetworkLocationSetStatus(NetworkLocation nl)
        {            
            try
            {
                Ping ipchecker = new Ping();
                PingReply reply = ipchecker.Send(nl.Address, 1000);
 
                if (reply.Status == IPStatus.Success && nl.IsAlive == false)
                {
                    writer.WriteLine(nl.Name + "[" + nl.Address + "] went alive at " + DateTime.Now);
                    nl.IsAlive = true;
                }
                else if (reply.Status != IPStatus.Success && nl.IsAlive == true)
                {
                    writer.WriteLine(nl.Name + "[" + nl.Address + "] went dead at " + DateTime.Now);
                    nl.IsAlive = false;
                }
            }
            catch (Exception)
            {
                writer.WriteLine(nl.Name + "[" + nl.Address + "] went dead at " + DateTime.Now);
            }
        }
now the thread method kept on submitting asynchronous begininvokes of NetworkLocationSetStatus(NetworkLocation nl) irrespective of how much time it took to finish up(a dead node takes up considerable time to finish up i.e. being declared dead). hence calls for NetworkLocationSetStatus(deadnode) kept piling up asnchronously and soon everything went haywire.
 
Solution was just to change the BeginInvoke to Invoke. that way the thread remains locked till NetworkLocationSetStatus() method finishes up and then the loop moves onto the next node.
 
Lesson learnt. And asynchronous methods are not all that great if you don't know what u are doing. For novice programmers like me, synchronous rocks (though its slow).
 
Thanks everybody for your support. Thanks a lot.
  Permalink  
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 1

Its problem in your this chunk of code:
 
while (true)
                {
                    NetworkLocation[] all_locations = new NetworkLocation[LocationList.Count];
 
                    LocationList.CopyTo(all_locations);
                    foreach (NetworkLocation nl in all_locations)
                    {
                        this.BeginInvoke(new nlss(this.NetworkLocationSetStatus), new Object[] { nl });
                    }
                    Thread.Sleep(1000);
                }
 
1. Your while does not break
2. BeginInvoke is a synchronous method. So basically your while is useless here. remove while.
 
it should be only foreach loop. Put delay inside that.
 
NetworkLocation[] all_locations = new NetworkLocation[LocationList.Count];
 
LocationList.CopyTo(all_locations);
foreach (NetworkLocation nl in all_locations)
{
    this.BeginInvoke(new nlss(this.NetworkLocationSetStatus), new Object[] { nl });
    Thread.Sleep(1000);
}
 
You are done!
  Permalink  
Comments
jibesh at 21-Jan-13 18:21pm
   
'BeginInvoke is a synchronous method. So basically your while is useless here. remove while.' No its wrong. BegingInvoke in Asynchronous.
Grasshopper.iics at 21-Jan-13 18:33pm
   
Put argument to rest, please check with the suggested changes first!
jibesh at 21-Jan-13 18:38pm
   
arhh.. you asked to remove the while too!! the usage of while is this method is a thread procedure and he is querying the status of each IP in a thread and having the Sleep outside this Foreach is good enough to introduce a delay. Also the CPU usage is because of the other while loop without delay inside the Logger class. Please check my solution.
Grasshopper.iics at 21-Jan-13 18:53pm
   
BeginInvoke by UI controls or thread is meant to be executed at the background. So When you have loads of statement and use BeginInvoke at the end, UI gets updated smoothly. Do a simple test. Take a Label, and a button. on click event write the following code.
 
for (int i = 0; i < 100; i++)
{
this.BeginInvoke((MethodInvoker)delegate
{
label1.Text = i.ToString();
System.Threading.Thread.Sleep(100);
 
// your UI update code here. e.g. this.Close();Label1.Text="something";
});
}
// System.Threading.Thread.Sleep(2000);
MessageBox.Show("Done");
 
Dont you see your UI in wait state? That is what I mean. In his code context, calling BeginInvoke from inside nested loops is not executing asynchronously. First Invoke is locking the resource, the second call is waiting for that to complete. So even though they are executing asynchronously, lock is not managed that way.
jibesh at 21-Jan-13 19:22pm
   
care to read this. also can you show which variable represents the UI control inside the while loop?
 
There are no UI involved here in this loop/thread context. The Foreach method executes every second or so and iterate through the IP List and check the status of the IP and write to the log file.
 
I dont see any relation between the sample you listed above and the one in discussion now. The begininvoke delegate invokes 'NetworkLocationSetStatus' method and there is no sleep inside method and what you are insisting here is delay the each IP ping by 1 sec. say i have 10 ips in the list, it takes each one sec to complete the ping operation regardless of the wait time for the Ping itself.
 
I hope you read OPs code thoroughly. before make further comments.
Rate this: bad
good
Please Sign up or sign in to vote.

Solution 2

Add a delay in your logger-WriteAsync method
 
private void WriteAsync()
{
     while (true)
     {
      try
      {
 
        String filename = Environment.CurrentDirectory +\\+ DateTime.Now.ToString("ddMMyyyy") + ".log";
         if (!File.Exists(filename))
              File.Create(filename);
 
         using (StreamWriter writer = new StreamWriter(filename,true))
         {
             String[] messagelistcopy = _messagelist.ToArray();
             _messagelist.Clear();
 
              foreach (String msg in messagelistcopy)
              {
                 writer.WriteLine(msg);
              }
              writer.Flush();
           }
         Thread.Sleep(10000); // Added 10 sec delay
      }
      catch (Exception)
      {
        //do nothing
      }
   }
}
 
Also I dont see any use of having two try-catch block in this method.
  Permalink  

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

  Print Answers RSS
0 George Jonsson 215
1 Kornfeld Eliyahu Peter 169
2 OriginalGriff 120
3 PIEBALDconsult 110
4 Zoltán Zörgő 99
0 OriginalGriff 6,165
1 DamithSL 4,658
2 Maciej Los 4,087
3 Kornfeld Eliyahu Peter 3,649
4 Sergey Alexandrovich Kryukov 3,294


Advertise | Privacy | Mobile
Web01 | 2.8.141220.1 | Last Updated 22 Jan 2013
Copyright © CodeProject, 1999-2014
All Rights Reserved. Terms of Service
Layout: fixed | fluid

CodeProject, 503-250 Ferrand Drive Toronto Ontario, M3C 3G8 Canada +1 416-849-8900 x 100