I am using Activz.Net 9, and want to async load multiple obj files, so I use multiple thread, but I will get GetConstructorInfo exception as following:
After some research, I found that the following simple code can reproduce the problem:
for (int i = 0; i < 3; i++)
{
Task.Run(() => { vtkUnsignedCharArray.New(); });
}
So, is there a problem with my usage? Are there any restrictions in vtk? I don’t need the multi-threading of the pipeline, I just load multiple obj files asynchronously.
The exception results from a concurrent access to the mummy internal table that stores the C# references to VTK objects.
Activiz is not thread safe, you can’t create objects with such an approach. However you can probably use a native C# array to load your data and then copy it to your vtkUnsignedCharArray in the main thread using marshalling.
Activiz is not thread safe, you can’t create objects with such an approach.
It used to be thread safe. It synchronised on the wrapped objects table. Has this changed?
/// <summary>
/// Factory method to create a wrapped object from a registered TypeEntry.
/// Client dlls that provide objects should register their known types via
/// RegisterType prior to any possible call to CreateWrappedObject.
/// </summary>
private object CreateWrappedObjectImpl(uint mteStatus, uint mteIndex, uint rawRefCount, System.IntPtr rawCppThis, bool callDisposalMethod, out bool created)
{
object obj = null;
if (null != this.WrappedObjectsTable)
{
lock(this.WrappedObjectsTable.SyncRoot)
{
obj = this.WrappedObjectsTable[rawCppThis];
System.WeakReference wr = obj as System.WeakReference;
if (null != wr)
{
obj = wr.Target;
}
}
}
if (null != obj)
{
created = false;
++WrappedObjectsTableHits;
}
else
{
created = true;
++WrappedObjectsTableMisses;
// Get a constructor info for the type of the object we're supposed
// to create via mteIndex:
//
System.Reflection.ConstructorInfo ci = GetConstructorInfo(mteIndex);
// Invoke the constructor with parameters:
bool strong = true;
if (0 == mteStatus || rawRefCount < 2)
{
strong = false;
}
object[] ctorParams = new object[3];
ctorParams[0] = rawCppThis;
ctorParams[1] = callDisposalMethod;
ctorParams[2] = strong;
obj = ci.Invoke(ctorParams);
}
return obj;
}
The code has not changed. The WrappedObjectsTable is locked but the IndexedConstructors is not. Adding lock (Instance.IndexedConstructors.SyncRoot) in GetConstructorInfo will probably fix the problem here, but I am not sure this is enough to make Activiz thread safe in general. Classes that use vtkGarbageCollector internally are not well handled by the C# GC.
is going out of scope before the constructor is called. Moving the constructor invocation inside the if block might fix that. You have a simple use case to test against.
If the Visualization pipeline is not thread safe, I think it might be ok. But if independent data objects are not thread-safe, the code will be difficult to write.
While I agree that what you propose should work, I am not sure to understand why you don’t think that locking IndexedConstructors is the best approach? To me, it is the object affected by the race condition, WrappedObjectsTable is not used at this point.
Anyway, I agree we have a simple test case to reproduce and it is worth having a look.
I’m not worried about this specific problem, I’m worried about whether there will be similar problems in other places, so that my vtk related code must be completely single-threaded.
Yes we advise that your VTK code must be single threaded in general, as there will be similar problems in other places. It does not mean that we can’t give it a try, and this specific problem must be fixed for that. Having completely independent filters that don’t share inputs, and that don’t use pipelines could potentially work. This should be investigated further to clearly identify what is supported and what is not.
Maybe @alexy.pellegrini has inputs?
Thanks for the clarification, I understand. I have tested some asynchronous scenarios, mainly asynchronously loading some mesh and color data using vtkPolyData for VTK. So far, except for this problem, other asynchronous scenarios are working very well.
While I agree that what you propose should work, I am not sure to understand why you don’t think that locking IndexedConstructors is the best approach?
Because the c# constructor adds the new object to the WrappedObjectsTable hash table. If another thread tries to create an object with the same underlying c++ instance, e.g. during deserialisation, a race condition will ensue. Holding the lock until after construction prevents the clash.
So that’s one problem eliminated.
However in the use case suggested by @liuzhongshu the error seems to be related to the way the IndexedConstructors array is being populated at runtime as wrapped objects are created. So locking on IndexedConstructors would fix that issue but a better solution would be to setup IndexedConstructors during application initialisation.
Synchronising on WrappedObjectsTable resolves both issues.
If VTK itself supports multi-threading with pipelines and shared inputs then I would expect the c# wrapper code should too.
VTK pipelines aren’t thread safe regardless of their inputs and outputs. This is due the VTK garbage collector pipeline related object (vtkAlgorithm, vtkInformation and vtkExecutive) because they hold strong cyclic references to each others.
VTK GC can only run in the main thread, and is called on every Delete() or those types, and some will fire internally on the Update() of the algorithm, due to information vectors being created and destroyed at some point.
As long as this GC is used, it will be UB to update, delete, and maybe even constructing, algorithms in multiple thread, or even in a thread that isn’t the main thread. Note: if you build in debug (without NDEBUG defined actually), you may trigger an assertion in VTK GC. There is an issue, opened a year ago by @jaswantp that is related to this exact problem.
In short: you can’t safely use VTK algorithm in multiple threads, regardless of their inputs and outputs. And this is a VTK issue. Other VTK classes does not have this limitation, and are safe to be create and destroyed in any thread AFAIK.
Activiz classes creation and registration should indeed be thread safe, and I agree that it should be considered a bug that it isn’t supported correctly.
In short: you can’t safely use VTK algorithm in multiple threads, regardless of their inputs and outputs. And this is a VTK issue. Other VTK classes does not have this limitation, and are safe to be create and destroyed in any thread AFAIK.
FYI, we’ve found a way to get around this limitation in MR vtk/vtk!8975. @berk.geveci iirc, we started working on it for async-paraview, then moved on to other priorities. Is there anything else needed on that to land?
@alexy.pellegrini Your description is slightly confusing so I want to clarify a little bit. You can use VTK pipelines in a multi-threaded way as long as all threads have completely different algorithms. The thread safety issue arises only when algorithms are shared across threads (and things are deleted) leading to potential garbage collection thread issues. Even after @jaswantp 's change, I would not recommend using the same algorithm in multiple threads (except maybe with locks and only to manage the state from one thread).
What @alexy.pellegrini and I are talking about is a thread safety issue at the C++ level. The C# issues discussed in this thread seem to be on top of the C++ issues.
Thank you so much for your insight @berk.geveci ! This is great news that things are moving forward in VTK.
I confirm the C# issue reported in this thread is on top of the C++ issues you mention, and it will be fixed in the next Activiz release.
Then, the VTK C++ issues have further impact in C#, where the GC is generally running in a background thread. Here disabling the VTK GC is the only way to prevent an undefined behavior where the C# GC deletes a pipeline while the VTK GC is checking for references. We will give a quick try to @jaswantp’s change to see if things are improved there.