Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When executing Async requests on multiple threads with 1 restclient, there is sometimes an exception #368

Closed
nickvane opened this issue Apr 4, 2013 · 0 comments

Comments

@nickvane
Copy link

nickvane commented Apr 4, 2013

When configuring a RestClient and using that client to execute multiple threaded executes, this works most of the time, but sometimes gives following exception:

System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
at System.Collections.Generic.List`1.Enumerator.MoveNext()
at RestSharp.RestClient.ConfigureHttp(IRestRequest request, IHttp http)
at RestSharp.RestClient.ExecuteAsync(IRestRequest request, Action`2 callback, String httpMethod, Func`4 getWebRequest)
at RestSharp.RestClient.ExecuteAsync(IRestRequest request, Action`2 callback)
at RestSharp.RestClient.ExecuteAsync[T](IRestRequest request, Action`2 callback)
at RestSharp.RestClientExtensions.ExecuteAsync[T](IRestClient client, IRestRequest request, Action`1 callback)

This happens because the defaultparameter collection is changed on each execute.
https://github.com/restsharp/RestSharp/blob/master/RestSharp/RestClient.Async.cs line 78

// add Accept header based on registered deserializers
var accepts = string.Join(", ", AcceptTypes.ToArray());
this.AddDefaultParameter("Accept", accepts, ParameterType.HttpHeader);

ConfigureHttp(request, http);

The ConfigureHttp method in https://github.com/restsharp/RestSharp/blob/master/RestSharp/RestClient.cs line 286

// move RestClient.DefaultParameters into Request.Parameters
foreach(var p in DefaultParameters)
{
    if(request.Parameters.Any(p2 => p2.Name == p.Name && p2.Type == p.Type))
    {
        continue;
    }
    request.AddParameter(p);
}

enumerates over the collection which could be changed while enumerating because an execute on another thread adds a parameter to the default parameter collection of the restclient.

I think that adding the accept headers in each request is not necessary. On the contrary, for each new request a parameter is added to the collection and this makes the collection constantly grow.
I would suggest moving the adding of the accept header out of the Execute method, although I don't immediately see another method where this would fit.

nickvane pushed a commit to nickvane/RestSharp that referenced this issue Apr 9, 2013
@ayoung ayoung closed this as completed Apr 12, 2013
haacked added a commit to haacked/RestSharp that referenced this issue Jun 20, 2013
Since anyone can directly reference DefaultParameters, it's possible for
a null value to be added. Definitely a programming mistake most likely,
but we seem to have run into this somehow. It might have been due to
weird multi-threading issues like those in restsharp#368 so I'm not sure if this
is still a problem. I figured it doesn't hurt to have defense in depth.
:)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants