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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent concurrent modification when iterating request commands #76

Merged

Conversation

lahma
Copy link
Contributor

@lahma lahma commented Sep 9, 2019

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following matches the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 馃懏.
  • I have provided test coverage for my change (where applicable)

Description

There was an unfortunate problem in optimization PR which surfaced in OrchardCore. Commands were mutated with active iterator and that broke the contract. This fixes the problem and hopefully you could do yet another release to NuGet 馃檹

@@ -157,7 +157,8 @@ public async Task Invoke(HttpContext context)
IDictionary<string, string> commands = this.requestParser.ParseRequestCommands(context);
if (commands.Count > 0)
{
foreach (string command in commands.Keys)
List<string> commandKeys = new List<string>(commands.Keys);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have used ToArray() here but a list will do for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dread to use System.Linq in active/hot path 馃槈

@JimBobSquarePants JimBobSquarePants merged commit 5f8107a into SixLabors:master Sep 10, 2019
@JimBobSquarePants
Copy link
Member

@lahma This is certainly critical enough to warrant a new release so here you go! 馃槃

https://www.nuget.org/packages/SixLabors.ImageSharp.Web/1.0.0-beta0009

@lahma lahma deleted the fix-collection-enumeration branch September 10, 2019 04:12
@lahma
Copy link
Contributor Author

lahma commented Sep 10, 2019

Thanks for the prompt action and release (and sorry for the extra chore)! 馃憤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants