Marten.PLv8.Patching API not thread safe?

[dustinmoris]

I am working with a client who uses the Marten.PLv8.Patching API in a multi-threaded application and we have seen a lot of unhandled exceptions in the logs which read like this one:

Unhandled exception. System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at System.Collections.Generic.Dictionary`2.TryInsert(TKey key, TValue value, InsertionBehavior behavior)
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at Marten.PLv8.Transforms.TransformSchema.AddFunction(TransformFunction function)
   at Marten.PLv8.Transforms.TransformSchema.Load(TransformFunction function)
   at Marten.PLv8.Transforms.TransformSchema.loadPatchDoc()
   at Marten.PLv8.Transforms.TransformSchema.schemaObjects()+MoveNext()
   at System.Collections.Generic.LargeArrayBuilder`1.AddRange(IEnumerable`1 items)
   at System.Collections.Generic.EnumerableHelpers.ToArray[T](IEnumerable`1 source)
   at Marten.PLv8.Transforms.TransformSchema.get_Objects()
   at Weasel.Core.Migrations.DatabaseBase`1.generateOrUpdateFeature(Type featureType, IFeatureSchema feature, CancellationToken token)
   at Weasel.Core.Migrations.DatabaseBase`1.ensureStorageExists(IList`1 types, Type featureType, CancellationToken token)
   at Weasel.Core.Migrations.DatabaseBase`1.EnsureStorageExists(Type featureType)
   at Marten.Storage.MartenDatabase.Marten.Storage.IMartenDatabase.EnsureStorageExists(Type documentType)
   at Marten.PLv8.Transforms.TransformExtensions.EnsureTransformsExist(IDocumentOperations operations)
   at Marten.PLv8.Patching.PatchingExtensions.patchById[T](IDocumentOperations operations, Object id)
   at Marten.PLv8.Patching.PatchingExtensions.Patch[T](IDocumentOperations operations, Guid id)
   at Program.<<Main>$>g__PatchItemAsync|0_2(IDocumentStore store, Guid itemId) in /Users/dustingorski/Documents/Temp/MartenApp/src/MartenApp/Program.cs:line 56
   at Program.<<Main>$>g__PatchItemAsync|0_2(IDocumentStore store, Guid itemId) in /Users/dustingorski/Documents/Temp/MartenApp/src/MartenApp/Program.cs:line 57
   at Program.<Main>$(String[] args) in /Users/dustingorski/Documents/Temp/MartenApp/src/MartenApp/Program.cs:line 47
   at Program.<Main>(String[] args)

The first thing that stood out to me was that it felt very odd that the Add method of an IDictionary<TKey, TValue> would throw a System.IndexOutOfRangeException:

System.IndexOutOfRangeException: Index was outside the bounds of the array.

After further investigating I learned that this is a very common error when a dictionary gets accessed from multiple threads at the same time. It is not a thread safe data structure which is why .NET has the ConcurrentDictionary inside System.Collections.Concurrent.

I created a small demo application to reproduce this error which can be found here:

https://github.com/dustinmoris/marten-test-app

I believe that the culprit is the TransformSchema class which has a local IDictionary where it stores all transformation functions.

This dictionary is accessed during a patch operation when loadPatchDoc() is being called. I am not 100% sure, but after browsing through the Marten code I believe that the reason why this dictionary is being shared and accessed via multiple threads is because an instance of type TransformSchema is stored inside the StoreOptions object and then loaded via this static extension method, essentially making the TransformSchema a singleton because if I'm not mistaken the StoreOptions is also a singleton as part of the singleton IDocumentStore:

https://github.com/JasperFx/marten/blob/master/src/Marten.PLv8/Transforms/TransformExtensions.cs#L23

However, I am a little bit out of my depth here if I'm honest. I don't have too much experience with Marten and I might be misunderstanding some core concepts here or we might have used the Patching API in a way which it was never intended to be used. Anyhow, I'm happy to help with the investigation or even help with a PR if you think that I've found an issue indeed.

Many thanks for this library and keep up the great work!

[oskardudycz]

Thank you, we'll have a look.
In most of the places, we're using ImHashMap from ImTools, which is faster than ConcurrentDictionary. It might be that we should also consider using it here.

@jeremydmiller FYI.

[jeremydmiller]

Ditto what @oskardudycz said. Let's switch that to use ImHashMap