[RESOLVED] collections being modified-multithreading

[cengineer]

I realize this has been covered a million times before but with threading sometimes a problem is app specific. I have a multithreaded app that assigns a url to a thread which it then fetches and crawls. Now the function where I collect each threads items I have added synclocks assuming this would avoid alterations. But I get the "collection was modified, enumeration operation may not execute" error.
Please look at my code:

 For Each landingPage As String In landingPages
                        'logger.constructLog("Working on landingPage link: " & landingPage & " at " & DateTime.Now.ToString, True)
                        Dim thread = New Thread(AddressOf processQuery)
                        count = count + 1
                        thread.Name = "Worm" & count
                        thread.Start(landingPage)
                        ThreadList.Add(thread)
                        If numThread >= 10 Then
                            Exit For
                        Else
                            numThread = numThread + 1
                        End If
                        'processQuery(landingPage)
                        logger.constructLog("Complete with link search: " & landingPage & " at " & DateTime.Now.ToString, True)
                    Next
...process function


If Not String.IsNullOrEmpty(html) Then
                If searchChoice = "something1" Or fromUrl.Contains("something1") Then
                    SyncLock something1lock
                        links = parsingUtilities.Getsomething1LinksFromHtml(fromUrl, html, searchItem)
                        posts = parsingUtilities.GetPsomething1FromHtml(links)
                    End SyncLock
                ElseIf searchChoice = "something2" Or fromUrl.Contains("something2") Then
                    SyncLock something2lock
                        links = parsingUtilities.Getsomething2LinksFromHtml(fromUrl, html, searchItem)
                        posts = parsingUtilities.GetPsomething2FromHtml(links)
                    End SyncLock
                ElseIf searchChoice = "something3" Or fromUrl.Contains("something3") Then
                    SyncLock something3lock
                        links = parsingUtilities.Getsomething3LinksFromHtml(fromUrl, html, searchItem)
                        posts = parsingUtilities.GetPsomething3FromHtml(links)
                    End SyncLock
                ElseIf searchChoice = "something4" Or fromUrl.Contains("something4") Then
                    SyncLock something4lock
                        links = parsingUtilities.GetsomethingLinksFromHtml(fromUrl, html, searchItem)
                        posts = parsingUtilities.GetPsomething4FromHtml(links)
                    End SyncLock
                End If


                Dim ExecutionError As String
                Try
                    SyncLock dlock

                        For Each l As String In links
                            finallinks.Add(l)
                        Next
                        For Each p As String In posts
                            finalposts.Add(p)
                        Next

                    End SyncLock

                Catch ex As ApplicationException
                    messagebox.show(ex.message.tostring)


                End Try


               
                Try
                    SyncLock dlock

                        Dim filepath As String = Environment.GetFolderPath(Environment.SpecialFolder.MyDocuments)

                        Dim outfile As New StreamWriter(filepath & "\Visual Studio 2008\Projects\output.txt", True)



                        Dim i As Integer = 0


                        For Each link As String In finallinks

                            While i < posts.Count()
                                postlink = finalposts.Item(i)
                                i = i + 1

' ...the rest is code to enter data into a database

Also I forgot to mention that this entire process that includes the creation of threads for the url fetching is being run by a background thread also. I needed an individual thread to run this processes since the main thread needs to handle something else.
I thought I would post the part where I create the threads and assign them since I think there is an issue here as well, I




I know the code is poorly done...so please offer suggestions.
Thanks

[cicatrix]

You lock the wrong objects.
It's the collection that should be locked for the whole cycle of enumeration (For each...)

SyncLock links

SuncLock posts

etc.

[cengineer]

Thank you for the response. I understand what you are saying but not sure where in the code:

SyncLock something1lock
                        links = parsingUtilities.Getsomething1LinksFromHtml(fromUrl, html, searchItem)
                        posts = parsingUtilities.GetPsomething1FromHtml(links)
End SyncLock

These I separated since I thought each thread needs to go to my parsing class and do the brunt of the work in (getlinks) but having the links/posts locked up as I have...does that not nullify the point of threads. I still need a data structure to collect the links but did not want to assign one for each type. Maybe I should?
Do you think this is even necessary to copy the links and posts into different lists to output to database and then locking finallist and postlist again to do that?

Try
                    SyncLock dlock
                        For Each l As String In links
                            finallinks.Add(l)
                        Next
                        For Each p As String In posts
                            finalposts.Add(p)
                        Next
                    End SyncLock

It just seems my way of locking everything up does not result in faster processing.

or did you just mean locking links and post here:

 For Each link As String In finallinks..
       While i < posts.Count()
                                postlink = finalposts.Item(i)
                                i = i + 1

Unrelated question if you dont mind, I used a while loop here since I was having trouble with the nested for each loops since the two lists are not always the same size. Anyway thanks for your patience and help.

[cicatrix]

' You are enumerating finallinks and for the whole for...next loop your collection should be locked
SyncLock finallinks
    For Each link As String In finallinks 
        
        ' Here, you're enumerating posts and finalposts so:
        SyncLock posts
            SyncLock finalposts
                While i < posts.Count()
                    postlink = finalposts.Item(i)
                    i = i + 1
                 End While
            End SyncLock
        End SyncLock
    Next
End SyncLock

Each collection should be locked for the whole enumeration loop so it doesn't change by another thread.

Generally you can't add something or remove something from the collection and enumerate it at the same time. Either you modify it or iterate trhough it, you can't do both things simultaneously.

[cengineer]

Thanks