Downloading Files - weird problem

P

PiotrKolodziej

hi
I have a thread that downloades a file.
Problem is : Not all files are beeing downloaded. I observed that only the
small files are beeing downloaded correctly. I also cant download two files
in a row. I got to Exit and run the program again to even start downloading.
The second problem i interprett as iam not closing something ( but don't
know what ). For the first problem i have no idea what is wrong.Here is the
download thread
Maybe someone could help


try
{
HttpWebRequest request = null;
HttpWebResponse response = null;

request = (HttpWebRequest)WebRequest.Create(FullPath);
request.Headers.Clear();
request.ProtocolVersion = HttpVersion.Version11;

response = (HttpWebResponse)request.GetResponse();

FileStream = response.GetResponseStream();
// Download folder must work
file = new BinaryWriter(File.Create(@"d:\\FM\\" +
fileName));

while ((bufferCount = FileStream.Read(buffer, 0, 64000)) > 0
&& !Stop)
{
//MessageBox.Show("STOP is: " + Stop.ToString() + "
BufferCount is: " + bufferCount.ToString());
store += bufferCount;
file.Write(buffer, 0, bufferCount);
}

file.Close();
response.Close();
FileStream.Close();
file.Close();

if (!Stop)
{
NormalExit = true;
exitDelegate myDeleg = new exitDelegate(exitForm);
this.Invoke(myDeleg);
}
}
 
P

PiotrKolodziej

I found out that when i create instance of my Download class for a second
time the Close has the value from the first instance
Close is:

static bool stop;
static readonly object stopLock = new object();
static bool Stop
{
get
{
lock (stopLock)
{
return stop;
}
}
set
{
lock (stopLock)
{
stop = value;
}
}
}

Why does it happen? Shouldn't Dispose() clear the value of 'Close' ?
 
K

Kevin Spencer

Hi Piotr,

There are a couple of issues that I can see here. First, you're not ensuring
that the Files you create are closed in the case of an exception:

file = new BinaryWriter(File.Create(@"d:\\FM\\" + fileName));

while ((bufferCount = FileStream.Read(buffer, 0, 64000)) > 0 && !Stop)
{
//MessageBox.Show("STOP is: " + Stop.ToString() + "
BufferCount is: " + bufferCount.ToString());
store += bufferCount;
file.Write(buffer, 0, bufferCount);
}

file.Close();
response.Close();
FileStream.Close();
file.Close();

There are some things you can do to help this. Here is an example:
try
{
response = (HttpWebResponse)request.GetResponse();
using (file = new BinaryWriter(File.Create(@"d:\\FM\\" + fileName)))
{
while ((bufferCount = FileStream.Read(buffer, 0, 64000)) > 0 &&
!Stop)
{
//MessageBox.Show("STOP is: " + Stop.ToString() + "
BufferCount is: " + bufferCount.ToString());
store += bufferCount;
file.Write(buffer, 0, bufferCount);
}
}
}
catch
{
// ... whatever exception-handling/recording you want here
}
finally
{
response.Close();
}

A couple of notes about the above: The "using" statement ensures that the
file is closed, as soon as the "using" block is exited. Also, note that I
closed the HttpWebResponse, but not the FileStream. This is because closing
the HttpWebResponse also closes the FileStream, although explicitly closing
the FileStream doesn't cause any problems; it's just unnecessary. I put the
"response.Close()" statement into a "finally" block, which means that it
will be executed no matter what happens.

Also, I'm not sure whether you may have some memory issues associated with
the allocation of such a large buffer, but that's not necessarily a problem.

The main thing is, you need to handle any possible exception, ensure that
IDisposable instances are disposed, and close the HttpWebResponse,
regardless of any exceptions.

Is this in a loop by any chance?

--
HTH,

Kevin Spencer
Microsoft MVP
Chicken Salad Surgery

What You Seek Is What You Get.
 
P

PiotrKolodziej

A couple of notes about the above: The "using" statement ensures that the
file is closed, as soon as the "using" block is exited.
Click to expand...

Did you mean that if i use using directive i dont have to close file in ANY
situation
( normall exit or exception )?
closed the HttpWebResponse, but not the FileStream. This is because
closing the HttpWebResponse also closes the FileStream, although
explicitly closing the FileStream doesn't cause any problems; it's just
unnecessary. I put the "response.Close()" statement into a "finally"
block, which means that it will be executed no matter what happens.
Click to expand...

Thank you for that.
Also, I'm not sure whether you may have some memory issues associated with
the allocation of such a large buffer, but that's not necessarily a
problem.
Click to expand...

Everything is possible but this. I've Already tought this way.
The main thing is, you need to handle any possible exception, ensure that
IDisposable instances are disposed, and close the HttpWebResponse,
regardless of any exceptions.
Click to expand...

This is only the beta version of the code. I have planned to put some try
blocks into the code later.
Anyway your 'thread' with using directive is valuable for me.
 
K

Kevin Spencer

Did you mean that if i use using directive i dont have to close file in
ANY situation
( normall exit or exception )?
Click to expand...

Yes. The using block is the same as a try/finally block.

--
HTH,

Kevin Spencer
Microsoft MVP
Chicken Salad Surgery

What You Seek Is What You Get.
 
Top