views:

100

answers:

2

I download a file by e.g. 5 threads. When one of the threads completes downloading the file part - it is aborted, BUT all of the rest threads has the ThreadState = WaitSleepJoin and obviously stops downloading. How to resolve that ?

while ((bytesSize = responseStream.Read(Buffer, 0, Buffer.Length)) > 0)
{
    lock(fileStream)
    {
        fileStream.Write(Buffer, 0, bytesSize);

        if (DownloadedBytesCount >= EndPosition - StartPosition)
        {
            downThread.Abort();
            break;
        }
        else DownloadedBytesCount += bytesSize;
    }               
}

I suppose that the fileStream is still blocked after downThread.Abort(). I thought that the break unlocks the file stream but it's not. So how to unlock that file ?

Here You have some more info:

I have a class "ThreadFileManager":

public class ThreadFileManager
{
    public ThreadContent[] threads;
    protected static FileStream fileStream { get; set; }

    public ThreadFileManager(string fileUrl, string LocalPath, int ThreadCount, int bufferLength, ProgressBar progressBarReference)
    {
        if (File.Exists(LocalPath))
            fileStream = new FileStream(LocalPath, FileMode.Append, FileAccess.Write);
        else fileStream = new FileStream(LocalPath, FileMode.Create, FileAccess.Write);

        CreateThreads(fileUrl, ThreadCount, bufferLength); //create threads and start downloading
    }

    private void CreateThreads(string fileUrl, int ThreadCount, int bufferLength)
    {
        webRequest = (HttpWebRequest)WebRequest.Create(fileUrl);

        webRequest.Credentials = CredentialCache.DefaultCredentials;

        webResponse = (HttpWebResponse)webRequest.GetResponse();
        responseStream = webResponse.GetResponseStream();

        //calculate the total bytes count to download per one thread
        long part = webResponse.ContentLength / ThreadCount;
        fileLength = webResponse.ContentLength;

        this.threads = new ThreadContent[ThreadCount];

        ThreadContent thr_cn = new ThreadContent(bufferLength, fileUrl);
        thr_cn.StartPosition = 0;
        thr_cn.EndPosition = part;

        threads[0] = thr_cn;

        for (int i = 1; i < ThreadCount; i++)
        {
            thr_cn = new ThreadContent(bufferLength, fileUrl);

            thr_cn.StartPosition = (i * part) + 1;
            thr_cn.EndPosition = (i + 1) * part;
            this.threads[i] = thr_cn;
        }
    }
}

public class ThreadContent : ThreadFileManager
{
    public long StartPosition { get; set; } //the Begining position of the downloading file
    public long EndPosition { get; set; } //the End position of the downloading file
    public byte[] Buffer { get; set; }

    HttpWebRequest webRequest { get; set; }
    HttpWebResponse webResponse { get; set; }
    long BufferLength { get; set; }
    long DownloadedBytesCount { get; set; }

    Thread downThread;
    string FileURL { get; set; }

    public ThreadContent(int bufferLength, string url)
    {
        Buffer = new byte[bufferLength];
        downThread = new Thread(new ThreadStart(Download));
        FileURL = url;
    }

    public void Download()
    {
        int bytesSize = 0;
        webRequest = (HttpWebRequest)WebRequest.Create(FileURL);

        webRequest.Credentials = CredentialCache.DefaultCredentials;

        webResponse = (HttpWebResponse)webRequest.GetResponse();
        responseStream = webResponse.GetResponseStream();

        webRequest.AddRange(StartPosition, EndPosition);

        while ((bytesSize = responseStream.Read(Buffer, 0, Buffer.Length)) > 0)
        {
            lock (fileStream)
            {
                fileStream.Write(Buffer, 0, bytesSize);
                base.UpdateProgress(bytesSize);
            }

            if (DownloadedBytesCount >= EndPosition - StartPosition)
            {
                downThread.Abort();
                break;
            }
            else DownloadedBytesCount += bytesSize;
        }
    }
+3  A: 

In general, your approach is not correct because you shouldn't ever end your thread by calling Abort(). This ends up raising an exception. The approach I would try is to just allow the thread to return instead of aborting. You could do this by passing the thread info about when to return. Another way (which is probably better) is to use an AutoResetEvent, and Set() it when you want each thread to end. The code inside of the thread would have something like:

AutoResetEvent e = new AutoResetEvent( false);
while( !e.WaitOne( 0)) {
  // your code here
}

It looks like you want these 5 threads to share information about the download status, i.e. how many bytes have been downloaded by each thread. If you want to have real-time updating (which I assume), you could pass a reference to the count into each thread, and ensure that each thread uses a lock to protect access to this variable. Or you can have it only update the byte count every 1K or more, so that you're not locking and unlocking a bunch.

If you have a worker thread that manages these multiple downloading threads, then you can start each thread and then use EndInvoke from the "main" worker thread (i.e. not main thread itself or your GUI will block) to make sure each thread has ended properly. Or you can use an AsyncCallback, which gets called when each thread is done. Within the AsyncCallback, you can call EndInvoke.

Dave
@Henk I see... I've always read that calling Abort() in general is a bad idea, so I never do it. The code he posted doesn't look like the thread code, though. It looks like it's the "main" thread code that manages the other threads.
Dave
After all, it appears downThread == current thread.
Henk Holterman
@Henk yep, after he updated his code, it does look like downThread is the current thread, so Abort() is okay.
Dave
+3  A: 

Edit, after seeing the rest of the code

Thread.Abort() is not the issue here, but it is completely unnecessary. If you remove it the break whill end your loop, release the lock and end the Thread.

I see a few problems, do with this what you want:

  • I'm missing the calls to Thread.Start(). Are you sure any download Thread is started at all? It could be in code not shown but otherwise this would explain all other threads being WaitSleepJoin not running.

  • All output goes to a single static fileStream. There is no code to set the position of that stream, I think your output will consist of several out-of-order chunks.

  • ThreadContent should not derive from ThreadFileManager. A manager has several download-threads (composition) but not: a download-thread is a special case of Filemanager (inheritance).

  • the downThread variable confuses thing, inside Download() it always equals Thread.CurrentThread.

  • you're better of using the ThreadPool instead of creating your own threads.


I think your problem is elsewhere.

According to MSDN,

When a thread calls Abort on itself, the effect is similar to throwing an exception

and that means your lock statement is exited and the lock will be released.

But locking on the fileStream itself is not recommended, and I can't picture how you handle the Position in this shared file.

Henk Holterman
@Henk would you recommend that Tony have a prior "agreement" about which thread gets which portion of the file, have each thread download that portion without locks, and then at the end concat all of the pieces? I think that's what I would try first. But he'll still have to protect the DownloadedBytesCount if he wants any sort of reasonable progress information for the user.
Dave