How do I avoid memory leak in Unity when subscribing to c# event in referenced serialized ScriptableObject?

Question

During the implementation of my game using Unity I was faced with the following setup:

• I have a ScriptableObject (as an asset) that has a c# event delegate.
• I have a MonoBehaviour that has a serialized reference to the ScriptableObject that has the delegate.

I want to "subscribe" the MonoBehaviour to that ScriptableObject's event, properly handling the event to avoid memory leaks. Initially I supposed that subscribing to the event on the OnEnablecallback and unsubscribing it on the OnDisablewas enough. However, a memory leak occurs when a developer, by using the Unity Inspector, swaps the value of the serialized reference to the ScriptableObject during play.

Is there a canonical way to safely subscribe and unsubscribe to c# events in a serialized reference to a ScriptableObject, given that I want the developers of the game to be able to swap value in the inspector during play?

---

To illustrate that, I have written a simple code for that scenario:

SubjectSO.cs (The ScriptableObject with the event)

using UnityEngine;
using System;

[CreateAssetMenu]
public class SubjectSO : ScriptableObject
{
    public event Action<string> OnTrigger;

    public void Invoke()
    {
        this.OnTrigger?.Invoke(this.name);
    }
}

ObserverMB .cs (The MonoBehaviour that wants to subscribe to the event in the ScriptableObject)

using UnityEngine;

public class ObserverMB : MonoBehaviour
{
    public SubjectSO subjectSO;

    public void OnEnable()
    {
        if(this.subjectSO != null)
        {
            this.subjectSO.OnTrigger += this.OnTriggerCallback;
        }
    }

    public void OnDisable()
    {
        if(this.subjectSO != null)
        {
            this.subjectSO.OnTrigger -= this.OnTriggerCallback;
        }
    }

    public void OnTriggerCallback(string value)
    {
        Debug.Log("Callback Received! Value = " + value);
    }
}

InvokesSubjectSOEveryUpdate .cs (Auxiliary MonoBehaviour, for testing)

using UnityEngine;

public class InvokesSubjectSOEveryUpdate : MonoBehaviour
{
    public SubjectSO subjectSO;

    public void Update()
    {
        this.subjectSO?.Invoke();
    }
}

For testing, I have created two assets of the type SubjectSO, named:

• SubjectA
• SubjectB

Then, I have created a GameObject in scene, and attached the following components:

• ObserverMB, referencing SubjectA
• InvokesSubjectSOEveryUpdate, referencing SubjectA
• InvokesSubjectSOEveryUpdate, referencing SubjectB

When hitting play, the message Callback Received! Value = SubjectA is printed in the Console every Update, which is expected.

Then, when I use the inspector to change the reference in ObserverMB from SubjectA to SubjectB, while the game is still playing, the message Callback Received! Value = SubjectA still keeps being printed.

If I disable and enable ObserverMB in the inspector, both messages Callback Received! Value = SubjectA and Callback Received! Value = SubjectB start being printed every Update.

The initial callback subscription is still in effect, but, as a subscriber, ObserverMB has lost the reference to that event.

How can I avoid that situation?

I really believe that this seems to be a common use scenario for the use of c# event delegates and ScriptableObjects and it seems strange for me that OnEnable and OnDisable do not properly handle the serialization case of a developer tweaking the inspector.

Answer 1

Well you would have to check whether the subjectSO is being changed and unsubscribe in this case.

After you switch the value via the Inspector your class cannot unsubscribe from the previous value anymore. So whatever was subscribed to at the beginning will stay subscribed.

For checking on runtime

I would e.g. do it using a property like

// Make it private so no other script can directly change this
[SerializedField] private SubjectSO _currentSubjectSO;

// The value can only be changed using this property
// automatically calling HandleSubjectChange
public SubjectSO subjectSO
{
    get { return _currentSubjectSO; }
    set 
    {
        HandleSubjectChange(this._currentSubjectSO, value);
    }
}

private void HandleSubjectChange(SubjectSO oldSubject, SubjectSO newSubject)
{
    if (!this.isActiveAndEnabled) return;

    // If not null unsubscribe from the current subject
    if(oldSubject) oldSubject.OnTrigger -= this.OnTriggerCallback;

    // If not null subscribe to the new subject
    if(newSubject) 
    {
        newSubject.OnTrigger -= this.OnTriggerCallback;
        newSubject.OnTrigger += this.OnTriggerCallback;
    }

     // make the change
    _currentSubjectSO = newSubject;
}

so every time some other script changes the value using

observerMBReference.subject = XY;

it automatically first unsubscribes from the current subject and then subscribes to the new one.

---

For checking a change via the Inspector

There are two options:

Either you go via the Update method and yet another backing field like

#if UNITY_EDITOR
    private SubjectSO _previousSubjectSO;

    private void Update()
    {
        if(_previousSubjectSO != _currentSubjectSO)
        {
            HandleSubjectChange(_previousSubjectSO, _currentSubjectSO);
            _previousSubjectSO = _currentSubjectSO;
        }
    }
#endif

Or do (thanks zambari) the same thing in OnValidate

#if UNITY_EDITOR
    private SubjectSO _previousSubjectSO;

    // called when the component is created or changed via the Inspector
    private void OnValidate()
    {
        if(!Apllication.isPlaying) return;

        if(_previousSubjectSO != _currentSubjectSO)
        {
            HandleSubjectChange(_previousSubjectSO, _currentSubjectSO);
            _previousSubjectSO = _currentSubjectSO;
        }
    }
#endif

Or - since this will happen only in the case the field is changed via the Inspector - you could implement a Cutsom Editor which does it only in case the field is changed. This is a bit more complex to setup but would be more efficient since later in a build you wouldn't need the Update method anyway.

Usually you put editor scripts in a separate folder called Editor but personally I find it is good practice to implement it into the according class itself.

The advantage is that this way you have access to private methods as well. And this way you automatically know there is some additional behavior for the Inspector.

#if UNITY_EDITOR
    using UnityEditor;
#endif

    ...

    public class ObserverMB : MonoBehaviour
    {
        [SerializeField] private SubjectSO _currentSubjectSO;
        public SubjectSO subjectSO
        {
            get { return _currentSubjectSO; }
            set 
            {
                HandleSubjectChange(_currentSubjectSO, value);
            }
        }

        private void HandleSubjectChange(Subject oldSubject, SubjectSO newSubject)
        {
            // If not null unsubscribe from the current subject
            if(oldSubject) oldSubject.OnTrigger -= this.OnTriggerCallback;

            // If not null subscribe to the new subject
            if(newSubject) newSubject.OnTrigger += this.OnTriggerCallback;

            // make the change
            _currentSubjectSO = newSubject;
        }

        public void OnEnable()
        {
            if(subjectSO) 
            {
                // I recommend to always use -= before using +=
                // This is allowed even if the callback wasn't added before
                // but makes sure it is added only exactly once!
                subjectSO.OnTrigger -= this.OnTriggerCallback;
                subjectSO.OnTrigger += this.OnTriggerCallback;
            }
        }

        public void OnDisable()
        {
            if(this.subjectSO != null)
            {
                this.subjectSO.OnTrigger -= this.OnTriggerCallback;
            }
        }

        public void OnTriggerCallback(string value)
        {
            Debug.Log("Callback Received! Value = " + value);
        }

#if UNITY_EDITOR
        [CustomEditor(typeof(ObserverMB))]
        private class ObserverMBEditor : Editor 
        { 
            private ObserverMB observerMB;
            private SerializedProperty subject;

            private Object currentValue;

            private void OnEnable()
            {
                observerMB = (ObserverMB)target;
                subject = serializedObject.FindProperty("_currentSubjectSO");
            }

            // This is kind of the update method for Inspector scripts
            public override void OnInspectorGUI()
            {
                // fetches the values from the real target class into the serialized one
                serializedObject.Update();

                EditorGUI.BeginChangeCheck();
                {
                    EditorGUILayout.PropertyField(subject);
                }
                if(EditorGUI.EndChangeCheck() && EditorApplication.isPlaying)
                {
                    // compare and eventually call the handle method
                    if(subject.objectReferenceValue != currentValue) observerMB.HandleSubjectChange(currentValue, (SubjectSO)subject.objectReferenceValue);
                }
            }
        }
#endif
    }