Skip to content

Comments

Fix potential race condition in RequestService.ExecuteRequestTry method by capturing Request reference#910

Open
TManITtech wants to merge 2 commits intoabuzuhri:mainfrom
TManITtech:main
Open

Fix potential race condition in RequestService.ExecuteRequestTry method by capturing Request reference#910
TManITtech wants to merge 2 commits intoabuzuhri:mainfrom
TManITtech:main

Conversation

@TManITtech
Copy link
Contributor

Fixed error: Collection was modified; enumeration operation may not execute.

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 System.Linq.Enumerable.WhereSelectListIterator`2.MoveNext()     
at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)     
at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)     
at FikaAmazonAPI.Services.RequestService.LogRequest(RestRequest request, RestResponse response)     
at FikaAmazonAPI.Services.RequestService.<ExecuteRequestTry>d__39`1.MoveNext()  
--- End of stack trace from previous location where exception was thrown ---     
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)     
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)     
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()     
at FikaAmazonAPI.Services.RequestService.<ExecuteRequestAsync>d__43`1.MoveNext()  --- End of stack trace from previous location where exception was thrown ---     
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)     
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)     
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()     
at FikaAmazonAPI.Services.OrderService.<GetOrderItemsAsync>d__12.MoveNext()  
--- End of stack trace from previous location where exception was thrown ---     
at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)     
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)     
at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()     
at FikaAmazonAPI.Services.OrderService.GetOrderItems(String orderId, IParameterBasedPII parameterBasedPII)    

The System.InvalidOperationException: Collection was modified; enumeration operation may not execute error in LogRequest is caused by a race condition due to the RequestService class not being thread-safe.

Explanation

  1. Shared State: The RequestService class maintains a shared state property protected RestRequest Request { get; set; }.
  2. Concurrent Execution: Your application is likely making concurrent calls using the same instance of RequestService (or a derived service like OrderService).
  3. The Race:
  • Thread A executes a request and awaits the response in ExecuteRequestTry.
  • Thread B starts a new request on the same service instance. It overwrites the Request property with a new RestRequest object and begins adding parameters/headers to it (e.g., in RestHeader or AddAccessToken).
  • Thread A resumes after its await. It calls LogRequest(Request, response). Because it accesses the Request property directly, it picks up the new request object created by Thread B, not the one it actually executed.
  • The Crash: Thread A iterates over request.Parameters inside LogRequest (specifically the .Select(...).ToList() part) to log them. Simultaneously, Thread B is modifying that same Parameters collection (adding headers/params). Modifying a collection while enumerating it throws this exception.

Solution

To fix this, you should capture the Request object into a local variable inside ExecuteRequestTry before awaiting the execution. This ensures that LogRequest operates on the specific request that was sent, rather than whatever happens to be in the shared Request property when the task resumes.

@sonarqubecloud
Copy link

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

Successfully merging this pull request may close these issues.

1 participant