0

I'm trying to fix this issue from 2-3 hours but unable to fix it. I'm trying to download a file in three parts three threads. the problem is when one part is completed, other threads stop downloading.

Example: let's say i want to download 300kb 
part1->t1->100kb
part2->t2->100kb  //if this thread get completed then other two become unresponsive.
part3->t3->100kb     

Code i'm working with (Modified and shorter one but addresses my issue )

//inside a function
            WebResponse wresp = wreq.GetResponse();
            long e1 = wresp.ContentLength / 3;
            long e2 = 2*e1;
            long e3 = wresp.ContentLength;
            wreq.Abort();
            wresp.Close();
            wreq = null;
            wresp = null;
            byte[] buff1 = new byte[1500];
            byte[] buff2 = new byte[1500];
            byte[] buff3 = new byte[1500];
            HttpWebRequest hr1 = (HttpWebRequest)WebRequest.Create(textBox1.Text);
            hr1.AddRange(0, e1-1);
            WebResponse wresp1 = hr1.GetResponse();
            HttpWebRequest hr2 = (HttpWebRequest)WebRequest.Create(textBox1.Text);
            hr2.AddRange(e1,e2-1);
            WebResponse wresp2 = hr2.GetResponse();
            HttpWebRequest hr3 = hr1;//(HttpWebRequest)WebRequest.Create(textBox1.Text);
            hr3.AddRange(e2,e3);
            WebResponse wresp3 = hr3.GetResponse();
            Stream response1 = wresp1.GetResponseStream();
            Stream  response2 = wresp2.GetResponseStream();
            Stream response3 = wresp3.GetResponseStream();
            Stream f1, f2, f3;
            f1 = File.Create("Part1");
            f2 = File.Create("Part2");
           f3 = File.Create("Part3");
            int bytesRead=0, bytesProcessed=0;
            long total1=e1, total2=e2-e1, total3=e3-e2;
            int x1=0, x2=0, x3=0;
            Thread t1 = new Thread(() =>download(hr1, wresp1, buff1,response1,f1,bytesRead, bytesProcessed,total1,x1));
            t1.Name = "1";
            Thread t2 = new Thread(() => download(hr2, wresp2, buff2, response2, f2, bytesRead, bytesProcessed,total2,x2));
            t2.Name = "2";
            Thread t3 = new Thread(() => download(hr3, wresp3, buff3, response3, f3, bytesRead, bytesProcessed, total3,x3));
            t3.Name = "3";
            t1.Start();
            t2.Start();
            t3.Start();
            }

        }
        }

    private download(HttpWebRequest hr2, WebResponse wresp2, byte[] buff, Stream response, Stream f,int bytesRead,long bytesProcessed,long total,int x)
    {
        do
        {
            lock (lockerObj)
            {
                bytesRead = response.Read(buff, 0, buff.Length);
                bytesProcessed += bytesRead;
                f.Write(buff, 0, bytesRead);
                x = Convert.ToInt32(Thread.CurrentThread.Name) - 1;
                pb[x].Invoke((Action)delegate
                {
                    pb[x].Value = Convert.ToInt32(bytesProcessed * 100 / total);

                });
                if (bytesProcessed >= total)
                {
                    Thread.CurrentThread.Abort();
                    break;
                }
                lb[x].Invoke((Action)delegate
                {
                    lb[x].Text = "Downloaded " + Convert.ToDouble((bytesProcessed * 100 / total)).ToString() + "%";
                    label4.Text = "thread" + (x + 1);
                });

            }
        } while (bytesProcessed<=total && bytesRead>=0 );
    }
    static readonly Object lockerObj=new Object(); //at global level

The application ui remains responsive and from debugging i realized that each thread is exiting at same time. So the question is why the other threads are not completing their parts.

enter image description here

Arpit
  • 12,767
  • 3
  • 27
  • 40
  • 3
    BytesRead and BytesProcessed are being shared by reference between the 3 threads? causing your while() to fail on download after 1 thread completes? – Kyle Pittman Nov 05 '13 at 20:36
  • @KylePittman Nope; it's written in a confusing manor, but the value of those variables is copied to the parameter's storage when the method is called. Each thread has their own copy of the variable. The code is very misleading about it though. – Servy Nov 05 '13 at 20:41
  • @KylePittman but it's a simple call by value i guess.. – Arpit Nov 05 '13 at 20:42
  • 1
    You really shouldn't be locking over all of the important work; it pretty much defeats the purpose of creating multiple threads. Only one is ever doing anything besides sitting there doing nothing at any given point in time. As it is, you can't even have one working if another is stalled out and blocking on a read. – Servy Nov 05 '13 at 20:43
  • it's better to try this on another site and be sure about your server security rules. – BRAHIM Kamel Nov 05 '13 at 20:45
  • 3
    Also, you're trying to download a *single* file. That can't really be parallized at all. Three threads reading from the same stream isn't faster than one thread; the network is the bottleneck. And making three different network requests so that three threads can each read a third of the data is just going to slow you down as the bottleneck (the network connection) has to do three times the work. – Servy Nov 05 '13 at 20:46
  • @Servy actually i got this idea from IDM.. so do you think simple `webclient` will be equally or more efficient in downloading the single file? – Arpit Nov 05 '13 at 20:49
  • 1
    @Arpit It will be much easier on you, it will actually work, and yes, it will probably perform better. (Which is moot, if your solution doesn't even work. Performance is only worth comparing when both solutions *work*.) – Servy Nov 05 '13 at 20:52
  • try to download the file with IDM if this work there is no reasons to not work with your own code – BRAHIM Kamel Nov 05 '13 at 20:53
  • @BRAHIMKamel it's working in IDM – Arpit Nov 05 '13 at 20:53
  • @Servy i'm using webclient for non-resumable downloads. I'm trying this version for resumable downloads. Any idea of IDM trick ? – Arpit Nov 05 '13 at 20:54

2 Answers2

4

You have this condition at the end of the loop in your download method:

while (bytesProcessed<=0 && bytesRead>0 );

That's your problem. The first time through the loop, bytesProcessed is incremented by the number of bytes read, which makes it greater than 0, which causes the condition to be false and the loop exits.

You'll never do more than one iteration in the loop.

By the way, not every response will have a valid ContentLength. Web pages regularly provide no Content-Length header, in which case the ContentLength property will be -1. Even if the page does provide a Content-Length header, there's no guarantee that it's correct. So in the general case you can't guarantee that you've received the entire contents.

As Servy pointed out in his comment, splitting the download among multiple threads is unlikely to improve performance and fact could very likely cause the download to go slower. That's especially true if a site sees you making three concurrent requests and decides to rate limit or block you. In general you're better off doing a normal download with a single HttpWebRequest. It's more reliable, it simplifies your code, and probably will perform better than the overly complex multithreaded version you've coded.

Jim Mischel
  • 131,090
  • 20
  • 188
  • 351
  • Sorry but that was typo, the actual code is `while (bytesProcessed<=total && bytesRead>=0 );` – Arpit Nov 05 '13 at 21:58
  • Working with single `httpWebRequest` will be my last resort but i am curious about why the threads are exiting at same time. Doing some mor debugging, t1 and t2 are running perfectly together but the problem is with t3. is it a right to do `HttpWebRequest hr3 = hr1; hr3.AddRange(e2,e3);` – Arpit Nov 05 '13 at 22:02
  • No, it's not okay to do `hr3 = hr1;` You end up making two requests from a single `HttpWebRequest` object. The result of doing that will be unpredictable. Why you insist on using multiple requests when it's clear that a single request is the better solution is beyond me. But it's your code and your time to waste. Have fun. – Jim Mischel Nov 05 '13 at 22:18
-3

I agree with @Kyle Pittman. It's most probably BytesRead and BytesProcessed variables are the cause of your problem.

  • 4
    Nope, they're *copied by value* when calling the method each time. – Servy Nov 05 '13 at 20:44
  • Where is this the declaration of this variables? From where it is accessible? I think it's worth check this before anything. And, please, undo this negative vote on my answer, I'm trying to help him. – Michel Vaz Ramos Nov 05 '13 at 20:52
  • 2
    Why don't you look in the code and see for yourself? It's right there in the code. If you don't even know where the variable is declared then why are you claiming to know the answer to the question? It is your responsibility as the person answering to prove that your answer is correct. Explain to us *exactly* how these variables are causing the problem. I've downvoted your answer *because it's wrong*, and also because it's unhelpful and doesn't explain the actual error or how to fix it. If you fix it to actually be *correct*, then I would consider reversing my vote. – Servy Nov 05 '13 at 20:54
  • Actually, in his code there is no declaration of the variables bytesRead and BytesProcessed, that's why I tell him to first look on those variables. You are wrong: I'm not claiming to know the answer to the question, but it is my responsibility to help him to find the answer or find ways so he can find the answer by himself. Exactly at this point you failed by downvoting my help. I guess you click faster than you think. – Michel Vaz Ramos Nov 20 '13 at 14:52
  • Those variables are parameters to the method. You only need to look at the method signature to see them. If you're really struggling, just use ctrl+f to help you find them. Sadly, due to the poor formatting of the code, you'll need to scroll to the right to see them, but they are indeed there. While it's understandable to miss it, and clearly you're not the only one who did at first, once several others found it you really should have taken a much closer look. – Servy Nov 20 '13 at 14:57
  • Next, if you don't know the answer, but just want to add some info, you should be *posting a comment*, not an answer. Answers are exclusively for *answering the question*. If you want to add anything that's *not* an answer, then you should be posting a comment, not an answer. – Servy Nov 20 '13 at 14:58