Fundamental bug in bindings' lifecycle (Underlying native object was deleted).

[EgorBo]

Let me explain it via a code snippet:

var node = new Node();
scene.Remove(node);
scene.Add(node); // Boom! Exception - underlying native object was deleted.

Now let's analyze it line by line:

var node = new Node();

under the hood new Node() calls Node_Node which creates a native object as a WeakRef (so refs==0). Also, UrhoSharp adds this object to an internal static cache as a WeakReference. So if we don't use the node we just created and it goes out of the scope - GC will collect it and during the collection will remove the underlying native objects.

scene.Add(node);

Urho3D internally increments the ref count as it now needs this object alive at least as long as it's a part of the scene. UrhoSharp will receive a callback from this AddRef call and fill will find this object in the internal cache and promote it to StrongReference - so GC won't even try to collect it until it is a part of the scene (has refs > 0).

scene.Remove(node);

Urho3D internally decrements the ref count (it was == 1) and since it becomes refs==0 - it deletes the native objects and sends a notification to UrhoSharp. UrhoSharp finds this object (which is "node") in the cache and removes it from there. Now it is still alive but the underlying native object is already deleted. Once it goes out of scope - GC collects it. But since we still in the scope and if we try to re-use it, for example, insert the node to the scene again (or move it to an another parent) - Urho3D won't be able to do it, the "real" node doesn't exist. So it throws an exception (Underlying native object was deleted).

The possible solution is to add a strong ref immediately to an object we create via C# even if we are not going to use it. So this initial strong ref will mean "C# keeps a reference to it". The downside of it is:

scene.RemoveComponent(heavyStaticComponent);

won't remove a heavy StaticModel object immediately (because we don't know, probably user wants it to be alive and will attach to a different node later) - the object will be deleted only with the next GC collection and most likely from the GC's point of view it will be a small managed object in most likely Gen2. With the current implementation - object is deleted synchronously.

[migueldeicaza]

I do not think it is a problem to delete objects asynchronously, this comes with the territory, and it is the way that other things work.

The problem is that heavyStaticComponent is alive as long as the user keeps a reference to it. The idiom should be that for those heavy objects, the user should call Dispose on them to severe the link between the objects - at that point we should unref, and we should clear the flag.

One problem I am struggling to understand is the way that our Runtime is keeping track of objects right now in the RefCountedCache, and I do not understand the logic behind using WeakReference or not (StrongRefByDefault). I would like to know the history behind the decisions there.

[EgorBo]

@migueldeicaza Dispose already works as you described. For example in the "HugeObjectsCount" sample I use dispose that removes the managed objects from the cache (but doesn't remove the native object because it's being used (it will check for refsCount) - so the Dispose in that case simply removes the managed proxy) BUT I don't have to call Dispose there - it's just a small optimization 🙂 . I used Dispose in that sample because there were too many objects there. The logic behind WeakReference is for the cases when we create a node but do not insert it anywhere and it goes out of scope - if it was added to the cache as a strong ref - it would live forever. Historically I didn't want to force users to use Dispose at all (because it doesn't feel very natural). So all heavy objects/components/textures are released once I remove them from the scene in the same thread. Speaking of that - Urho3D objects may send events or do some logic on "Release" so we need to "dispatch" the removal operation to the update-loop (next frame).

[migueldeicaza]

I think that if you take the ref on every object created, you will be fine with some scenarios requiring Dispose().