Thread stops doing its job

Question

We have a C# application that connects to a FTP server, downloads some files, disconnects, and after a certain amount of time (selected by the user through the UI) reconnects and repeats the process. We implemented this using BackgroundWorker, but we noticed that after running for a longer time, the program stopped logging its actions, both in the UI and the log file. At that point, there were no files for it to download, so we uploaded some and it resumed activity as if nothing had happened.

The problem was that the regular users had no way of knowing that the program was still working, so we decided to implement it using our own threading. We did a simpler program, to rule out any other problems, and this one only connects to the FTP and disconnects. It stopped displaying messages just like BackgroundWorker (one time after 2 hours, one time after 22 hours, without any pattern that we could find, and on a computer that did nothing else).

DoFTPWork += new DoFTPWorkDelegate(WriteFTPMessage);

FTPWorkThread = new Thread(new ParameterizedThreadStart(Process));

//seData is the FTP login info
FTPWorkThread.Start(seData);

and the FTP method is:

private void Process(object seData1)
{
    seData = (SEData)seData1;
    while (!stopped)
    {
        try
        {
            ftp = null;
            ftp = new FTP_Client();

            if (ftp.IsConnected)
            {
                logMessages += DateTime.Now + "\t" + "info" + "\t" + "Ftp disconnected from " + seData.host + "\r\n";
                ftp.Disconnect();
            }

            ftp.Connect(seData.host, 21);
            ftp.Authenticate(seData.userName, seData.password);
            logMessages += DateTime.Now + "\t" + "info" + "\t" + "Ftp connected to " + seData.host + "\r\n";

            error = false;
            logMessages += DateTime.Now + "\t" + "info" + "\t" + "Trying to reconnect in 5 seconds\r\n";
            System.Threading.Thread.Sleep(5000);
            SlaveEventArgs ev = new SlaveEventArgs();
            ev.Message = logMessages;
            txtLog.Invoke(DoFTPWork, ev);
            System.Threading.Thread.Sleep(200);
            logMessages = "";
        }

        catch (Exception ex)
        {
            logMessages = "";
            if (ftp.IsConnected)
            {
                ftp.Disconnect();
            }
            ftp.Dispose();
            logMessages += DateTime.Now + "\t" + "ERR" + "\t" + ex.Message + "\r\n";

            logMessages += DateTime.Now + "\t" + "info" + "\t" + "Trying to reconnect in 5 seconds\r\n";
            SlaveEventArgs ev = new SlaveEventArgs();
            ev.Message = logMessages;
            txtLog.Invoke(DoFTPWork, ev);
            System.Threading.Thread.Sleep(5 * 1000);
            error = true;
        }
    }
}

WriteFTPMessage displays the message in a TextBox and in the original program wrote to a .txt file.

Answer 1

If I'm understanding you correctly this while(!stopped) loop is the loop that is running for several hours? If that is the case, where are you terminating your ftp connection if anywhere? The only time you close it in the code you've posted is if an exception is thrown, otherwise you simply dereference the object and create a new one which is a pretty serious resource leak and at least contributing to the problem if not causing it.

Also it seems that ftp is globally accessible. Are you accessing it anywhere using a different thread? Is the object thread safe??

EDIT:

The biggest issue I see here is design. Not that I'm trying to bag on you or anything but you've got all sorts of operations intermixed. Threading, logging and ftp access code all in the same function.

What I would recommend is restructuring your program. Create a method much like the following:

// Called by thread
void MyThreadOperation()
{
   while(!stopped)
   {
      // This is poor design in terms of performance.
      // Consider using a ResetEvent instead.
      Thread.Sleep(5000);

      try
      {
         doFTPDownload();
      }
      catch(Exception ex)
      {
         logMessage(ex.ToString());
      }
   }
}

doFTPDownload() should be self contained. The FTP object should be created and opened inside the function when it is called and prior to it finishing it should be closed. This same concept should be applied to logMessage() as well. I would also recommend using a database to store log messages instead of a file so that locking issues don't complicate matters.

I know this isn't an answer in that you may still experience issues since I can't say for certain what could be the cause. However I'm confident with a little design restructuring you will be much better able to track down the source of the problem.