ScriptableObject oddities, are these expected behaviours ?

edit : finally submlitted as a bug : case 456419.

1) Removing a Scriptable object from a list does call destroy, but doesn't delete it

Is it expected behaviour ?
I checked, instantiating a ScriptableObject with an List.Add(ScriptableObject.CreateInstance()), and then performing list.Clear() does call OnDestroy and OnDisable, but never deletes it during the whole player lifecycle. (testing null on it returns false, and simply testing with FindAllObjectsOfType(myDerivedScriptableObject) whenever I stop the player).
Which results in leaks.

I'd find it odd to be expected ... ?

2) another ScriptableObject oddity, but this one is an advantage, is that if you call this code from inside this SObject :

Code (csharp):

1. Object.DestroyImmediate(this);
2. return "stuff";

It does return "stuff". I thought DestroyImmediate would destroy its instance instantly, therefore not allowing to call the next code line ? Anyway it's a positive unexpectation though, as it lets us create completely autonomous ScriptableObjects (destroys itself instantly but still allows us to pass its members to another class).
I'd like to know if this is a bug too, as I'm actually using it, and seeing it changed in a later release would break my game.

 

Just out of curiosity... how else could that have worked?

 

Maybe I'm confused, but DestroyImmediate() is supposed to well, "immediately destroy" an object, so I guess calling DestroyImmediate(this) would simply set "this" to null instantly ? Therefore forbidding any further code lines to be run ?
I admit I didn't test the basic c# class version, with let's say :

Code (csharp):

1.  
2. //inside another class
3. void Destroy(myClass instance){
4.  instance = null;
5. }
6.  
7. //and then inside any given method of myClass do :
8. void myClassDestroy() {
9. anotherClass.Destroy(this);
10. doStuff();
11. }
12.  

Logically doStuff() should never be run, or should it ? Am I wrong ?

 

In C# (and all other CLI based languages) an object will be freed when not one reference to it exists (look up the Garbage Collector docu when you want to know more). Unity will mark the Object as destroyed, this is only that Unity knows to no longer use this object.

Setting the argument instance to null, will not destroy or free the object, it will only remove one reference, the reference you put as argument into the method.

Destory() and DestroyImmediate() are not comparable to delete you may know from languages like C++.

This may be basically whats DestroyImmediate does (very simplified):

Code (csharp):

1.  
2. class Foo
3. {
4.  private bool isDestroyed = false;
5.  
6.   public void DestroyImmediate()
7.   {
8.     isDestroyed = true;
9.   }
10.  
11.   public Bar(int a)
12.   {
13.     if (isDestroyed)
14.       throw ObjectDestroyedException();
15.     // do other stuff
16.   }
17. }
18.  

This is also why the UnityEngine.Object has an implicit bool operator. The object may exist, but its marked as destroyed.

 

Thanks for the nice explanation Marrk, it was instructive.
Although, I still don't understand why the ScriptableObjects from the cleared list still exist ? They're only referenced in that list, so calling list.Clear() should clear them ? Therefore putting them to null ?

If it was only a matter of milliseconds to wait I wouldn't bother, but here they stay persistent over the whole player lifecycle, and I get the inevitable "Some objects were not cleaned up when closing the scene" message from Unity when I stop the player.

 

The GC will not immediatly delete the data. I wonder why OnDestroy will be called on the elements in the list, as you dont destroy them by simply removing them from the list (clearing the list).

I think Unity keeps an internal list for all Unity related objects, so that simply loosing all your references to the object will not lead to the destruction of it.

 

Ok, so basically, clearing the list just removes their reference in the list, but they still keep being alive as long as I don't Destroy them by direct pointing.

So I made a quick test about it, and the results are still hair snatching :

1) I removed any self destruction code from myScriptableObject, and moved any manual destruction to a global manager class. Just to make sure that everything's cleanly destroyed (avoiding post-destruction self references leaks, etc).
2) Then I still perform my list.Add(ScriptableObject.CreateInstance<myScriptableObject>())
3) after that, in a foreach loop, I'm parsing the list, and whenever the current myScriptableObject meets a particular condition (ex : myScriptableObject.test == true), it copies it to a new myScriptableObject instance. This part is important to understand my surprise after this repro case. So basically, I just do :

Code (csharp):

1.  
2. //*******
3. //insert any code to fill a list with list.Add(ScriptableObject.CreateInstance<myScriptableObject>()) here
4. //*******
5. //then :
6.  
7. myScriptableObject result = null;
8. foreach(myScriptableObject SO in list) {
9.             if (SO.test == true) { //pseudocode, imagine that in some cases, test can be false
10.                 result = GetCopyAndDestroy(SO);
11.             } else {
12.                 Object.DestroyImmediate(SO);
13.             }
14.         }
15. list.Clear();
16.  

GetCopyAndDestroy() does a deep copy, member by member, of the ScriptableObject, and then call DestroyImmediate() on it. So I can't be cleaner.

In every case, I'm destroying any reference, and any "solid" myScriptableObject instance that could be left in the wild.
But still ... when I stop the player, FindAllObjectsOfType(typeof(myScriptableObject)) returns me those very myScriptableObject instances ... I think I'm lost.

edit : the only possible outcome woud be that everytime I change the "result" reference to a new myScriptableObject, it does call OnDestroy on its old one, but doesn't delete it. If this is true (and I'm about to test it), this is very dangerous and leaky native behaviour. Logically, an object that have no reference should be deleted, am I right ?

 

Ok it's confirmed.
Assigning a new reference to a ScriptableObject variable does call OnDestroy(), but doesn't delete the old one (assuming this variable was the only reference left for the old one).
Holy crap, speak about opening the Gates of Leak Hell ...
Should I report this as a bug ?

(reminder : the leaked ScriptableObject is never deleted during the whole Unity player lifecycle)

edit : even more odd ..... so when I manually destroy the old variable reference before assigning a new myScriptableObject to it, performing FindObjectsOfType(myScriptableObject) doesn't find anymore of them (except that new one, of course). So it's good.
But ... when I stop the player, Unity still call OnDestroy() on what would be them (a Time.time value is contained in each one so I can trace them), and still tells me "Some objects were not cleaned up when closing the scene". What the hell ...

 

And how would that work?

I just called:

Code (csharp):

1. var resultString =randomInstance.DeleteMeImmediatelyFunction();

and now want to log the string. What happens?

 

In my test, it still assign resultString the right value. I thought it should throw a NullReference exception ? or make it just Null ?
Anyway this is not a big deal, I swapped the self destruction to a global manager. Now I got a serious problem with leaked ScriptableObjects remaining (see above), and I don't know why ...

 

So your understanding of references might need revision.

 

Care to ... elaborate ?
Are you saying that "randomInstance" is not a reference ?

Anyway, nevermind, back to the real problem.

 

I finally submitted a bug report about ScriptableObjects not being deleted. Will update the thread if I got some feedback (we can always dream a bit, can we ? ^^)

 

Let's take a look at your example:

Code (csharp):

1.  
2. //inside another class
3. void Destroy(myClass instance){
4.  instance = null;
5. }
6.  
7. //and then inside any given method of myClass do :
8. void myClassDestroy() {
9. anotherClass.Destroy(this);
10. doStuff();
11. }

myClass simply passes a copy of a reference to itself to the Destroy method. The Destroy method simply sets that copied reference to null. Nothing gets destroyed. The class is unaffected. Code won't 'magically' not run [which could cause major problems.]

The problems you face are because you think that you can destroy a class, and that an instance can be destroyed in the middle of execution. I suspect that as soon as you realize the only way to 'destroy something' is simply not reference it anywhere and let the GC take care of it, that you'll be significantly closer to fixing your memory leak.

I suggest you take a look at the following test case - step through it in VS: http://pastebin.com/HjSQJNgL

 

Ah, I see ... Thanks for taking the time, that's appreciated ;-)
What I was doing was a bit silly now that I understand ...

Although, I still don't get the leak after that, as I didn't use such a self destroying method anymore, but simply called Destroy from a global manager. Here is the bug I submitted :

Anyway I found a way to avoid the "bug" (?) by not filling the list with an instance creation anymore, but a flat class name reference instead, and then when the choice is made create that class from that name. No more leak, and it's lighter. But well, I'd be interested to follow that case as ScriptableObjects are widely used in general for their serializable properties.

 

did you check if the scriptable objects you found are still alive? or are they still there, but already marked as destroyed? Resources.UnloadUnusedAssets() (async) and GC.Collect() can help to get more cleaned up (but not perfect) view on memory usage.