Memory Access Violation in a Multithreaded component

I’m not sure either of these is guaranteed to be threadsafe. Print() certainly isn’t. I wrote that and it’s just based on List<T>.Add(), which is not threadsafe. CreatePlanarBreps should be in an ideal world, but I don’t know if it has been made threadsafe yet. I’ll have a look, but do not expect to learn much from it.

If you want to keep a log during a multithreaded process, you’ll need to use ConcurrentBag during and then print them all out afterwards.

From a quick reading I may not fully understand what you are doing, but it seems you are adding items to an index/item/key which doesn’t exist.

I have not used Dictionaries in .NET, and perhaps Dictionaries automagically adds missing items (keys) if entering them, but I wouldn’t expect it to do it in a threadsafe way. Especially when experiencing crashes…

As David suggests, collect your missing keys/items in a (Concurrent)Bag while inside the Parallel-loop and add them to the Dictionary later, outside the Parallel loop, then I bet you will have no problems.

Basic rule: Don’t write to variables (which changes) inside a parallel loop/process which are not definied inside that very loop/process. This is where ConcurrentBag comes handy in.

// Rolf

This line is to add a null in the list for this path, to pass null branches through the component. I’m not sure why this would be a problem, since ConcurrentDictionary can handle null values and is threadsafe. I have received null branch results just fine on many executions. I commented this out and still got crashes.

Also tried commenting out all the Print statements, and still got crashes. I have suspected for a while that the Brep.CreatePlanarBreps method itself is not threadsafe, which would explain a lot.

Marc

It’s too much complicated C++ code for me to dig through. But one thing you might as well try is to duplicate all your curves prior to sending them to the CreatePlanarBreps() method. Shot in the dark though.

Perhaps posting the exact exception you’re getting helps us in understanding where and how it goes wrong. I.e. are you sure it is the ConcurrentDictionary that causes the exception?

The exception Memory Access Violation almost surely points to a problem in unmanaged code. Which in turn means something goes wrong in the CreatePlanarBreps code. But that code does so much that it will be really hard to pinpoint.

Try running your code in V6. If things work in V6, then you know it is lower level code in core Rhino that has been tuned up to be thread safe.

No – not sure at all that it is the ConcurrentDictionary itself – I’m no expert at reading/understanding exceptions… in fact, the “TargetSite” here had led me to believe that perhaps it was the Brep method itself that was suspect, but I don’t really know – would appreciate some illumination on how to dig into an exception.

David – you point to a problem in unmanaged code… would wrapping this up in a proper VS component help, or at least help debug?

Steve – Great point – will try in Rh6, however… I believe you guys are multithreading components in GH1.0, so there would be no need for this code there, I assume…

Thanks everyone for jumping in on this.

Cheers,
Marc

Oh duh, the stacktrace:

“UnsafeNativeMethods” – is that the key piece of information there? I feel like I missed that the first time around…

I just think this is a quick way to determine if the threading problem is occurring in core Rhino. If the code works in V6, you’re probably out of luck for being able to multithread this in V5.

That is the transition point between .NET code and native code. Yes, it is a key piece of information

@stevebaer so, got around to testing this out in Rhino 6. One thing of note is that the C# component identified the method I was using as obsolete, and recommended using the method that uses a tolerance as the input. That said, the original code did not crash, and neither did the recommended method.

After changing the code:

Interestingly, the code I wrote is significantly faster than the GH 1.0 with MT enabled. Any thoughts on that?

Thanks for help debugging – I guess I’m going to have to abandon MT for this component in Rhino 5… not the end of the world, at least I can safely assume it’s not a problem with my code for now.

Thanks all!

Marc

Is it still faster after pressing F5 a few times?

Yep… still faster. Also – weird thing but the non-parallel native component is consistently faster than the parallel one. :stuck_out_tongue:

p.s. the component is yellow because it’s reporting a null branch (returned no results)

I wonder if there’s thread locking going on in the core code. If so, any attempts at multi-threading will just yield worse results than doing it straight.

I’m not going to worry too much about this, GH2 will have a completely different approach to parallelising components.

This is one of the reasons we provide the ability to choose if you want your component to run in single threaded or multithreaded modes. With very low execution time like you are showing, the set up time for creating tasks becomes a significant factor compared to the time it takes to perform that actual geometry computation.

1 Like

With many items and lower execution times you can sometimes squeeze out some more out of a Parallel.For if using a precalculated partitioner to avoid repetetive overhead calculations, like so:

            var partitioner = System.Collections.Concurrent.Partitioner.Create(0, your_array.Length);
            System.Threading.Tasks.Parallel.ForEach(partitioner, (range) =>
            {
                for (int n = range.Item1; n < range.Item2; n++)
                {
                     var obj = your_array[n];
                }
           }

// Rolf

We use Tasks instead of Parallel.For for the V6 multithreaded components.

Sounds wise, and gives more freedom to balance the workload. Parallel.For is only for us lazy scripters that just want to get things out of our hands. :slight_smile:

// Rolf