Race condition in BundleBase.RenderRawContent

[themikecom]

There is a race condition in BundleBase.RenderRawContent which causes an exception when the static Dictionary rawContentCache is manipulated by multiple threads.

Here is a test which will occasionally fail due to the race condition: https://github.com/themikecom/SquishIt/commit/a024c33032f55f15cea6d5e0c1ab86db9a17da39.

Here are a couple of fixes that come to mind:

• Add some locking to BundleBase.RenderRawContent
• Add a "Thread safe" Dictionary and use it in BundleBase
• Update SquishIt.Framework from .net 3.5 to 4 and replace the Dictionary with a System.Collections.Concurrent.ConcurrentDictionary. (May already be under way? https://groups.google.com/forum/#!topic/squishit/zM4X-crBF_o)

If you would like, I could submit a pull request with your choice of fix.

Thanks, Mike

[AlexCuse]

1.0 is delayed - I got a half baked PR that ripped out the system.web bits to a separate assembly but left the library in an unusable state. I need to keep it, but I also need to find the time to make it work again which is a challenge. In an effort to deliver some of the shit that people need decided to move what was slated for 1.0 except that change into 0.9.9 (all version numbers subject to change).

I would guess that standard rendering suffers from the same issue as well, would you mind taking a look? I'd prefer we do the same thing along both lines of code. Option 3 seems the most appealing to me. If you want to submit a PR can you do it against this branch: https://github.com/AlexCuse/SquishIt/tree/release/v0.9.9

If you get some time I'd love to chat about how you are using this. It might give me some ideas about how to slice the code up when "1.0" finally arrives.

[themikecom]

It looks like the commits I added pushed the error into BundleState.Assets. In-between work I'm trying to come up with a test case to repro the problem but it's not frequent and seems related to the other tests. The new error makes me think something is adding to bundleState.Assets while it is also being iterated over.

System.AggregateException : One or more errors occurred.
  ----> System.InvalidOperationException : Collection was modified; enumeration operation may not execute.
   at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout)
   at System.Threading.Tasks.Task.WaitAll(Task[] tasks)
   at SquishIt.Tests.LessTests.CanBundleCssWithAsNamedMultiThreaded() in C:\code\SquishIt\SquishIt.Tests\LessTests.cs:line 258
--InvalidOperationException
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource)
   at System.Collections.Generic.List`1.Enumerator.MoveNextRare()
   at System.Linq.Enumerable.WhereListIterator`1.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at SquishIt.Framework.Base.BundleBase`1.RenderRelease(String key, String renderTo, IRenderer renderer) in C:\code\SquishIt\SquishIt.Framework\Base\BundleBase.Rendering.Internals.cs:line 280
   at SquishIt.Framework.Base.BundleBase`1.Render(String renderTo, String key, IRenderer renderer) in C:\code\SquishIt\SquishIt.Framework\Base\BundleBase.Rendering.Internals.cs:line 155
   at SquishIt.Framework.Base.BundleBase`1.AsNamed(String name, String renderToFilePath) in C:\code\SquishIt\SquishIt.Framework\Base\BundleBase.cs:line 482
   at SquishIt.Tests.LessTests.<>c__DisplayClass10_0.<CanBundleCssWithAsNamedMultiThreaded>b__0() in C:\code\SquishIt\SquishIt.Tests\LessTests.cs:line 254
   at System.Threading.Tasks.Task.Execute()

As for usage, I'm using SquishIt in a multi-tenant web app to render the styling for a given domain using per-site settings and per-theme LESS. Every time a site owner updates their settings new styling is generated using RenderRawContent. The output is cached globally and also sent off in a ContentResult. This way only one node pays the render penalty and I can use the site id and theme id to build a URL that makes the CDN happy.

I had a single node that was throwing the error below like crazy. My guess is that a browser would request bundles a.min.css and b.min.css at the same time and cause the exception. Now the node is playing nice so it's going to be hard to confirm the fix.

System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
   at SquishIt.Framework.Base.BundleBase`1.RenderRawContent(String bundleName) in c:\dev\urandom\SquishIt\SquishIt.Framework\Base\BundleBase.cs:line 579
...

Eventually I'll build a proper update queue and rendering service but this gets it shipped.

[AlexCuse]

Yeah this stuff can be really hard to chase down. An environment like yours probably gives the best shot. Do you want to leave it running with your update for a while and see if it turns up anything before merging this in and putting on nuget?

[themikecom]

We deployed the modified v0.9.9 today. I also decreased the cache time so it'll get a little more exercise. We don't have a ton of traffic but I'll follow up if I see any errors.

[AlexCuse]

Awesome - I'll keep my fingers crossed! Circle back with me in a week or so and we'll get this merged in and onto nuget.

You can get my email address from the git logs if you need it, I forget if google groups makes it public or not.

[AlexCuse]

Has this been running with no ill effects?

[themikecom]

👍

I only see one dotless error in the stream over the last 7 days. I haven't looked into it because I've been moving .less files around and I figured it was something I did.

System.IO.FileNotFoundException: You are importing a file ending in .less that cannot be found [products.less].
File name: 'products.less'
   at dotless.Core.Importers.Importer.Import(Import import)
   at dotless.Core.Parser.Tree.Import.GetImportAction(IImporter importer)
   at dotless.Core.Parser.Tree.Import.Evaluate(Env env)
   at dotless.Core.Utils.NodeHelper.ExpandNodes[TNode](Env env, NodeList rules)
   at dotless.Core.Parser.Tree.Root.Evaluate(Env env)
   at dotless.Core.Parser.Tree.Root.AppendCSS(Env env)
   at dotless.Core.Parser.Infrastructure.Output.Append(Node node)
   at dotless.Core.Parser.Infrastructure.Nodes.Node.ToCSS(Env env)
   at dotless.Core.LessEngine.TransformToCss(String source, String fileName)
   at dotless.Core.CacheDecorator.TransformToCss(String source, String fileName)
   at dotless.Core.ParameterDecorator.TransformToCss(String source, String fileName)
   at dotless.Core.ParameterDecorator.TransformToCss(String source, String fileName)
   at SquishIt.Less.LessPreprocessor.Process(String filePath, String content) in C:\code\SquishIt\SquishIt.Less\LessPreprocessor.cs:line 51
   at SquishIt.Framework.Base.BundleBase`1.<>c__DisplayClass90_0.<PreprocessContent>b__0(String cntnt, IPreprocessor pp) in C:\code\SquishIt\SquishIt.Framework\Base\BundleBase.Rendering.Internals.cs:line 68
   at System.Linq.Enumerable.Aggregate[TSource,TAccumulate](IEnumerable`1 source, TAccumulate seed, Func`3 func)
   at SquishIt.Framework.Base.BundleBase`1.<>c__DisplayClass89_0.<PreprocessArbitrary>b__0() in C:\code\SquishIt\SquishIt.Framework\Base\BundleBase.Rendering.Internals.cs:line 59
   at SquishIt.Framework.Files.DirectoryWrapper.ExecuteInDirectory[T](String directory, Func`1 innerFunction) in C:\code\SquishIt\SquishIt.Framework\Files\DirectoryWrapper.cs:line 17
   at SquishIt.Framework.CSS.CSSBundle.<>c__DisplayClass32_0.<AggregateContent>b__0(Asset a) in C:\code\SquishIt\SquishIt.Framework\CSS\CSSBundle.cs:line 117
   at System.Linq.Enumerable.<SelectManyIterator>d__16`2.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at SquishIt.Framework.CSS.CSSBundle.AggregateContent(List`1 assets, StringBuilder sb, String outputFile) in C:\code\SquishIt\SquishIt.Framework\CSS\CSSBundle.cs:line 117
   at SquishIt.Framework.Base.BundleBase`1.GetMinifiedContent(List`1 assets, String outputFile) in C:\code\SquishIt\SquishIt.Framework\Base\BundleBase.Rendering.Internals.cs:line 333
   at SquishIt.Framework.Base.BundleBase`1.RenderRawContent(String bundleName) in C:\code\SquishIt\SquishIt.Framework\Base\BundleBase.cs:line 573
... controller fluff

[AlexCuse]

I just pushed this to nuget, not sure what index lag is like right now but should be available soon: https://www.nuget.org/packages/SquishIt/0.9.9-beta2

Thanks for all your help. If you think this is good to go, go ahead and close.

[themikecom]

Thank you!