Click here to Skip to main content
15,867,870 members
Please Sign up or sign in to vote.
1.00/5 (1 vote)
See more:
I came across very strange problem when programming in C# .NET:

There is a WebBrowser control, which loads some websites, in my program and a button.

At the very beginning, in button click event I'm starting thread this way:
C#
Thread oThread = new Thread(new ThreadStart(StartFunction));
oThread.Start();


Now, inside StartFunction I'm doing this:

C#
// Here is the beginning of conversation for which we are waiting
        public void StartFunction()
        {
            // Here is the beginning of conversation for which we are waiting
            bool Connected = false;
            while (!Connected)
            {
                HtmlElementCollection elc = null;

                this.Invoke((MethodInvoker)delegate
                {
                    elc = webBrowser.Document.Body.All;
                });

                foreach (HtmlElement element in elc)
                {
                    if (element == null || element.InnerHtml == null)
                    {
                        continue;
                    }

                    // determine what is it?
                    if (element.InnerHtml.StartsWith("connected"))
                    {
                        Connected = true;
                    }
                }
            }
         }


The exception ArgumentOutOfIndex is thrown on foreach loop, because the elements get changed while inside the loop.

Solution:
It seems that the best way to copy those items is to freeze them by running them on thread of WebBrowser (so they can't get changed).

C#
List<HtmlElement> collection = null;
Action CopyAction = () =>
{
    collection = new List<HtmlElement>(webBrowser.Document.All.Count);

    foreach (HtmlElement element in webBrowser.Document.All)
    {
        collection.Add(element);
    }
};
webBrowser.Invoke(CopyAction);


Now you can just manipulate on 'collection' without worrying that the elements get modified.
Posted
Updated 18-Apr-16 13:10pm
v3

Here is a link to help explain what Sergey is trying to have you understand about what is going on with the closure capturing the variable (not the value!)
Captured Closure (Loop Variable) in C# 5.0 - Stack Overflow[^]
It's interesting to note that, since C# 5.0 switching the
C#
for (int i = 0; i < elc.Count - 1; ++i)
{
    HtmlElement element = null;
    // EDIT: clarify that Invoked delegate assignment is removed with code below.
    this.Invoke((MethodInvoker)delegate
    {
        element = elc[i];
    });

to be
C#
foreach (HtmlElement element in elc)
{
solves both the captured variable and indexing errors!
(This only barely qualifies as a "Solution", but there's no good way to markup code in a comment.)

===========
Edit Apr 11, 2016 -- MTH:
Since the collection is getting changed out from under you at inopportune times, you need to get a copy of the collection in a safer way.
(This is all in addition to Sergey's comments suggesting that you need to rethink the strategy in general!)
Running the check in the UI thread should prevent the collection from being changed while a checking "pass through the collection" is in progress.
C#
// Here is the beginning of conversation for which we are waiting
public void StartFunction()
{
    // Here is the beginning of conversation for which we are waiting
    bool Connected = false;
    Action connectedCheck = () => {
           Connected = webBrowser1.Document.Body.All.Cast<HtmlElement>().Any(el =>
             {
               bool conn = false;
               if (el != null)
               {
                 string inner = el.InnerHtml;
                 if (inner != null)
                   conn = inner.StartsWith("connected");
               }
               return conn;
             });
         };

    while (true)
    {
      // connectedCheck should be quite fast.
      // There should be no reason that it can't run in the UI thread.
      // (Which should prevent the collection from being modified while it is running.)
      webBrowser1.Invoke(connectedCheck);
      if (Connected)
        return;
      // Now wait for something that indicates a change has happened on the web page.
      // Don't just run this check "open loop"!!
    }
}
 
Share this answer
 
v3
Comments
[no name] 7-Apr-16 15:28pm    
I think it is not that bad. Maybe not perfect in your opinion, that's why I only vote a 5 and not more!
Matt T Heffron 7-Apr-16 15:29pm    
Thanks!
EnGiNe.Pl 8-Apr-16 16:47pm    
I'll be clear - this solution doesn't work. It makes it happen less often but doesn't solve the problem.
Matt T Heffron 8-Apr-16 18:38pm    
On which line is it failing?
Is it still failing with an index out of range exception, or something else?

The HtmlElementCollection.GetEnumerator() copies from the source collection to a new array and iterates over that, so unless the source collection is changing in the middle of the GetEnumerator executing, I don't see how the foreach can fail.
Matt T Heffron 8-Apr-16 18:41pm    
And, I had omitted that the Invoke() is also to be removed when switching to the foreach.
Here is the idea: invoke is always required, because you do everything not in UI thread. You don't even need to check InvokeRequired — it will always return true.

With the browser instance, this is the same story, even if you use non-UI members. You created it in one thread, and use, for example, its indexed property (by taking .[index]) in another thread. If you create it in UI thread, also delegate the call to UI thread.

Be careful though. Too much synchronization may indicate that the whole design is wrong. It's possible that you can delegate so much to UI thread, you may make the situation worse than you could have not using that oThread at all. I'm not so sure, based on your information; this is just some food for thought for you. If this is the case, I would be probably able to suggest better design only if I had a lot more information on what you want to achieve.

[EDIT]

C#
// This is wrong:
for (int i = 0; i < elc.Count; i++)

// should be:
for (int i = 0; i < elc.Count -1; ++i)

Use the debugger properly.

—SA
 
Share this answer
 
v2
Comments
EnGiNe.Pl 6-Apr-16 11:01am    
I have tried it:
this.Invoke((MethodInvoker)delegate
{
element = elc[i];
});

but still the same exception... ArgumentOutOfRange
Sergey Alexandrovich Kryukov 6-Apr-16 11:33am    
Because you need to review the whole thing in holistic manner. I just gave your the idea.
Argument out of range another thing. Range of what? It that i value? How you pass it? It's not passed in the invoked anonymous function. Yes, through closure, I understand. But look at the consequences. Do you understand how closure works? Did you look at the range and the out-of-range value under the debugger? Do it, and you will understand the issue.

But I would suggest you review the whole design. I explained why in my answer.

—SA
EnGiNe.Pl 6-Apr-16 12:39pm    
for (int i = 0; i < elc.Count; i++)
{
HtmlElement element = null;
element = elc[i];

In the debugger: value is 103 and it should be between 0 and 101, the value of elc.Count can't be evaluated even if I followed your advice and put that invoke inside the code.
Sergey Alexandrovich Kryukov 6-Apr-16 12:46pm    
You again don't show how it's all passed to Invoke. It's the lack of important context. Anyway, elc.Count is 102, the index cannot go beyond 0..101. Why you mention one thing and not another one.

I'm saying this because this is one very usual problem, getting a value through closure. Again, do you understand it? Say, you pass value through the closure, but the time of execution is not the time when you create an anonymous method. By the time of call, the value is different. Do you understand the issue in general? If you do, you can fix the problem...

—SA
EnGiNe.Pl 6-Apr-16 15:49pm    
I thought I got it... but now I'm not sure at all - You said, get indexed property on UI thread, that's what I did - but it didn't resolve the problem at all. I don't get why the hell the value can't be evaluated at the time the debugger throws an exception?! What other code should I post?

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900