0

I have a ListBox with a list of URLs.

I have 2 threads taking theses URLs and treat them into a function.

My Thread 1 takes the items[0] of the ListBox, and my Thread 2 takes the items[1].

After the Thread picked up the item, it immediately remove it using Items.RemoveAt(0 or 1)

My problem using this method is that some of the URL are treated twice, some even not.

Isnt there a way to flag an URL or something else ? I'm not so familiar with multi threading

PS: In my example i said i was using 2 threads, in reality i use 5 threads.

Thanks in advance

EDIT : Used the concurentqueue system :

    Thread th1;
    Thread th2;
    Thread th3;
    Thread th4;
    Thread th5;
    ConcurrentQueue<string> myQueue= new ConcurrentQueue<string>();
    Int queueCount = 0;

    private void button2_Click(object sender, EventArgs e)
    {
    //initialize objects and query the database
        DBconnect conn;
        conn = new DBconnect();
        string query = "SELECT Url FROM Pages WHERE hash = ''";
        List<string> result = conn.Select(query);
        for (int i = 0; i < result.Count(); i++)
        {
    //For all rows found, add them to the queue
            myQueue.Enqueue(result[i]);
        }
    //start the 5 threads to process the queue              
        th1 = new Thread(ProcessTorrent);
        th2 = new Thread(ProcessTorrent);
        th3 = new Thread(ProcessTorrent);
        th4 = new Thread(ProcessTorrent);
        th5 = new Thread(ProcessTorrent);
        th1.Start();
        th2.Start();
        th3.Start();
        th4.Start();
        th5.Start();

    }


    private void ProcessTorrent()
    {
    //Start an unlimted task with continueWith
        Task tasks = Task.Factory.StartNew(() =>
        {
    //Check if there are still items in the queue
            if (myQueue.Count > 0)
            {
                string queueURL;
                bool haveElement = myQueue.TryDequeue(out queueURL);
        //check if i can get an element from the queue
                if (haveElement)
                {
        //start function to parse the URL and increment the number of items treated from the queue
                    get_torrent_detail(queueElement);
                    Interlocked.Increment(ref queueCount);
                    this.Invoke(new Action(() => label_total.Text = (myQueue.Count() - queueCount).ToString()));

                }
            }
        });
    //continue the task for another queue item
        tasks.ContinueWith(task =>
        {
            ProcessTorrent();
        });
    }
yves
  • 250
  • 1
  • 2
  • 18
  • 1
    Only the main thread (GUI Thread) can access GUI components like a ListBox. If you use background threads to accesss a GUI component, it will fail, eventually. – Pete Jan 04 '13 at 19:22
  • You should use a `ConcurrentQueue`. – SLaks Jan 04 '13 at 19:23
  • `if (myQueue.Count > 0)` is very wrong. Get rid of it. – SLaks Jan 06 '13 at 16:34
  • Your label text is also wrong. You should store the original number of items in a separate field. – SLaks Jan 06 '13 at 16:35

2 Answers2

3

It sounds like you're using a UI control to coordinate tasks between multiple threads.

That is an extremely bad idea.

Instead, you should queue up the tasks into a ConcurrentQueue<T> or BlockingCollection<T>, and have other threads take items from the queue and process them.

SLaks
  • 868,454
  • 176
  • 1,908
  • 1,964
  • Nothing fundamentally bad having multiple background workers, you know. Eslecially using a modern UI where the list is not "a UI control" but the list you were data binding into the UI. – TomTom Jan 04 '13 at 19:28
  • 1
    @TomTom: That's not what I said. He should not use a ListBox for this. – SLaks Jan 04 '13 at 19:28
  • @SLaks thx for the info, i edited my post with the solution ! – yves Jan 05 '13 at 00:36
-2

Yes, that happens because oyu do not synchronize access to the list.

Basically read the documentation C#, LOCK statement. Put up a lock while accessing the list. That prevents multiple threads from accessing it at the same time.

Then you ALWAYS get the top item (items[0]) immediately removing it.

I'm not so familiar with multi threading

I really love when people show that attitude. Can you imagine a cook, working in a restaurant as a professional cook, saying "ah, I am not familiar with an oven, you know". Or a doctor saying "ok, I have a problem here, I have no real idea how to give an injection". Given that today we live in a multicolored world, this sentence just SCREAMS in a bad way.

TomTom
  • 61,059
  • 10
  • 88
  • 148
  • 3
    Blindly using locks to create "thread-safe" code is just as bad. – SLaks Jan 04 '13 at 19:23
  • I never said blindly. I provided hints for someone "Professional" to read the documentation and read. – TomTom Jan 04 '13 at 19:25
  • 4
    God forbid someone is learning how to do multithreading and mentions that they're not real familiar with it. If he were an expert in it, he wouldn't need to ask the question. – Pete Jan 04 '13 at 19:25
  • 1
    you probably mean multi-cored, but, yes, we are multi-colored as well...however that applies. Plus, for a GUI developer it's perfectly fine not to know anything about threading...that is the unfortunate state of things for a lot of people who have been doing business applications – Alex Jan 04 '13 at 19:26
  • Yeah, in a forum for professional programmers. You REALLY miss that part. MultiThreading is not DirectX or another rarely used part. Multi Threaded is basic knowledge these days. Do not have it and you are stuck in a world where a phone has 2 cores minimum. – TomTom Jan 04 '13 at 19:27
  • 1
    @Alex Crap. Sorry. ESPECIALLY A UI developer, given that every operation taking more than 0.1 seconds should happen on a separate thread, should know about locks, multi threaded and access the UI Thread via dispatcher, so that he does not make applications hanging. That is baseline knowledge for UI code these days. – TomTom Jan 04 '13 at 19:27
  • "given that every operation taking more than 0.1 seconds should happen on a separate thread" - What do you base that on? – Pete Jan 04 '13 at 19:29
  • 1
    Usability. 0.1 seconds was 10 years ago and is still the timeframe win which the user starts getting disturbed. My old rules - since before 2000: 0.1 seconds: separate thread, 1 second: block UI visually and provide some progress indication - either a bar, at least a spinning symbol, depending on how long it supposedly WILL take, so the poor user is not left totally in the dark and clicks on butting like the application is crashed. Note that "feedback" CAN be "block the button" which SOMETIMES makes sense. But this is the old usability rule of not having applications that "somehow hang". – TomTom Jan 04 '13 at 19:31
  • @Pete The fact that performing an operation for any longer than that in the current thread would block the UI thread, thus freezing the application causing a horrible user experience. Also, this isn't *recent* in the UI world. For UI developer understanding threading has been important since day 1 for that exact reason. – Servy Jan 04 '13 at 19:32
  • Yea that doesn't make sense TomTom, if a cook isn't familiar with an oven then he/she obviously isn't a cook and wouldn't be in the kitchen to begin with, unless he/she was learning how to be a cook which in this case is our friend yves :) – TMan Jan 04 '13 at 19:34
  • And it leasds to funny things. "The application crashed" support calls. Users clicking on buttons until "they respond". End users are REALLY good doing funny things in the time your UI is busy doing other things. Heck, I have applications where EVERY callback HAS to be threaded because the UI is constantly updating. Video playback (blocks in the window while the UI is busy), financial updates. Non-trivial UI is half about getting out of the UI loop. Or - multi threaded UI's with a thread per window. – TomTom Jan 04 '13 at 19:35
  • If the app hangs for 0.1 seconds, I'm fairly sure my users aren't going to think it's hung. And this site isn't JUST for professionals. Read the first sentence of the FAQ. No need to dress people down for not being experts in every facet of programming. – Pete Jan 04 '13 at 20:07
  • @TomTom I appreciate your input and i will look forward between your advice and Slaks's one. About your comment on my knowledge, that's true. This is because i'm a web developper (PHP, Javascript, SQL and CSS). I used to do a bit of VB5 and C in the past but nothing complex as what i'm trying to do right now. FYI i started to write this c# application 5 days ago and i NEVER EVER used c# before..! I know some basics about objects, states and so but most of what i'm experiencing is new ;) – yves Jan 04 '13 at 22:51