Possible Threading, Variable copy issue

Question

We have an application that runs a routine like:

protected string _largeFile;

Execute()   //this executes in basically an infinite loop.
{
    _largeFile = "";
    DoStuff();
    DoStuff();
    _largeFile = "ReallyBigString"; //sometimes 4-5Mb
      if(_largeFile != "")
      {
       RaiseEvent();
      }
    DoStuff();
    DoSTuff();
    Thread.Sleep(5000);  //wait 5 seconds;
//repeat
}

CatchEvent()
{
     SendLargeFileToWebService(_largeFile);
}

This code executes on a client PC that we can't control. The code basically gets a large file and sends it back to our server. The issue is that sometimes the largeFile returned to the web service is blank. We have researched this for some time, and have been unable to determine how this happens.

The only solution that seems to have any merit is that the time it takes to RaiseTheEvent is taking so long that when the Execute methods is executing a subsequent time, the class level _largeFile variable is cleared before the SendLargeFileToWebService is able to do this.

My question: Is this even plausible? The developer that wrote the code defends that the reason for the class level variable was to avoid having to copy an instance variable and pass it around to the new thread (presumably the thread the event executes on). Does that seem like the right approach? There may not be a silver bullet answer to this, so if anyone can explain to me some standard for instance-event arguemnts vs. class variables when raising events I would appreciate it. I am also curious if the proposed issue is even plausible. Are their know issues when evaluating large strings from different threads.

Answer 1

I'm a little unclear about the scenario. But from your description it seems the Execute method is called from multiple threads. If so then yes you certainly have a bug because if at any time two Execute methods are running you will have a race condition over _largeFile.

There are 2 ways to fix this

1. Pass the contents of _largeFile as a parameter to Execute
2. Add a lock inside the execute method to prevent multiple threads from executing it at the same time.

The argument that they didn't want to pass the variable around smacks of laziness and IMHO should not be counted as a valid argument.

Answer 2

So, Execute clears _largeFile, does some stuff, maybe re-loads _largeFile. IF reloaded, it notifies another thread. That other thread then spends some time sending _largeFile across the web or something. Meanwhile, Execute can do some other stuff, then wait 5 seconds, then repeat.

Is That correct?

If so, then it is possible that _largeFile gets cleared (back at the top of Execute) before Send happens. But, well there are some buts:

1. I assume SendLargeFileToWebService(_largeFile); copies the value of _largeFile (ie copies the value of the pointer, doesn't copy all the contents)? If so, then you are saying the _largeFile is cleared before SendLarge... even starts. Given that Execute does some stuff, then waits 5 seconds, this does seem odd. But wait, maybe SendLarge... is busy sending an old file, so by the time it gets to the next file, it has missed its chance? ie The Catch is really in a loop right?:

   for_ever()
   {
       while (!nothing_to_catch)
           Wait();
       // !oh, caught something
       CatchEvent(); // calls SendLarge... which could take a while
   } // repeat
   

So by the time CatchEvent comes around to check for another event, _largeFile has been reset.

Or does each CatchEvent() happen in a new thread? I doubt it.

1. I can't remember the other 'buts' because I think the above is the problem, even though I don't know .NET well, etc.

Anyhow, try - upping your Sleep() time (just as a test - not as a solution!) - put a lock around _largeFile (again, as a test) or pass it around

And, check - does it tend to happen after a number of large files are sent 'back to back'? That would suggest my #1 scenario - ie the Send thread is getting so far behind that it eventually gets more than 5 seconds behind, and loses the file. In theory it could also skip a file and go to the next one. Does that happen?

And lastly, I read you question a number of times before I even felt that I might understand what you were trying to describe - like the other answer, I found it unclear. So if I am guessing wrong about the question, then, hey, I'm probably guessing the wrong answer as well.