Node removed from Scene can't be attached back to the scene

[gleblebedev]

If I call

BoxNode.Remove();

and then

Scene.AddChild(BoxNode);

the app crashes:

An unhandled exception of type 'System.AccessViolationException' occurred in TestViewportChange.dll

Additional information: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.

   at Urho.Node.Node_AddChild(IntPtr handle, IntPtr node, UInt32 index)
   at Sample.HandleKeyUp(KeyUpEventArgs obj) in E:\Temp\TestViewportChange\Scripts\Sample.cs:line 88
   at Urho.UrhoEventAdapter`1.<>c__DisplayClass5_0.<AddManagedSubscriber>b__0(TEventArgs args)
   at Urho.Input.<>c__DisplayClass213_0.<SubscribeToKeyUp>b__0(IntPtr x)
   at Urho.UrhoObject.ObjectCallback(IntPtr data, Int32 stringHash, IntPtr variantMap)
   at Urho.Application.Application_Run(IntPtr handle)
   at TestViewportChange.Program.Main(String[] args) in E:\Temp\TestViewportChange\VisualStudio\WinForms\Program.cs:line 33
   at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
   at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
   at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()

[andrekoehler]

This might be related to https://github.com/xamarin/urho/issues/285 Seems like UrhoSharp does not care if C# still holds references to Urho objects.

[migueldeicaza]

Good catch, the Remove method will destroy the underlying object if no Urho references are kept to it, from the documentation:

Remove from the parent node. If no other shared pointer references exist, causes immediate deletion.

I think we should catch this scenario, and either (a) detect this scenario and clear the handle, so we can throw an ObjectDisposedException(), or manually take a reference to the object before calling Remove (likely only if the reference count is one), and wait for the finalizer to do its job to dispose later.

A short term workaround for your scenario is to call AddRef before you call Remove

[EgorBo]

Indeed, when you create an instance of let's say Node via var node = new Node(); it uses WeakPtr under the hood so once you add this node to a scene - Urho3D automatically converts it to a SharedRef with Refs=1. Once you remove it - Refs=0 -> delete. UrhoSharp should create all native objects as SharedPtrs with Refs=1 (means C# keeps a strong reference) and release this ref on ~Finalize. I tried to make it work but without luck 😢 The downside of this approach is that native objects will live longer (as long as the managed objects are not collected by the GC and it will be GC's responsibility to notify the native side that those orphans can be deleted).

I guess at least now I should improve the error text

[migueldeicaza]

What was the issue you ran into with your approach?

An alternative is to handle it the way I suggested, manually flag methods to do this, which should only be a handful, and take a ref before we call into them.

[EgorBo]

@migueldeicaza I spent too much time fixing different kind of issues. I'll try once again later. Regarding the Node - there is a method ChangeParentthat can be used to safely change parent - https://github.com/xamarin/urho/blob/master/Bindings/Portable/Node.cs#L102-L105 Also, it turned out that direct "AddChild" call will also remove the old parent internally.

[EgorBo]

Moved to https://github.com/xamarin/urho/issues/302