Remove "Property.ValueChanged" subscribers when context holder is destroyed

Issue #104 resolved
Trinidad Sibajas Bodoque created an issue

When some object/s bind to a Property, a subscriber is added to its ValueChanged event, apparently caused by the ContextHolder component (indirectly), since for each one of them a new subscriber is added. However, the subscriber is never removed when the ContextHolder is destroyed. The subscribers should be removed in that situation, to allow them and all objects they reference to be properly destroyed and collected.

I assume it’s not necessary when the Context itself contains those properties, but if the properties are elsewhere they won’t be cleaned. As a use case example, the properties could be inside a persistent ScriptableObject asset, with the Context just “pointing” to them with C# properties; every time an instance of a prefab with a ContextHolder is created, or the scene is changed, etc., a new listener would be added to each property and never removed.

Comments (10)

  1. Christian Oeing repo owner

    @Trinidad Sibajas Bodoque

    Thanks for reporting! Could you send me a small test case where this issue occurs? With that I can probably quickly identify the location where I have to remove the listener.

  2. Trinidad Sibajas Bodoque reporter

    I’m sorry, I’ve been flooded with work these days, I’ll send it as soon as I can get to it.

  3. Trinidad Sibajas Bodoque reporter

    I have attached the test project. The "TestContext" contains a "mainColor", whose property is obtained from a "StyleData" asset in "Assets/Resources". To test it:

    • Open the "Test 1" scene.
    • Select "Assets/Resources/Style data", and press "Get Subscribers Count", which gets the number of subscribers to the "ValueChanged" event of the main color property using reflection. It will say it has 1.
    • Press the "Reload scene" button in the game to reload the scene, and check again the number of subscribers, which now will be 2. Every time the scene is reloaded the number of subscribers is increased, without ever decreasing (it would also happen when loading different scenes).

  4. Christian Oeing repo owner

    @Trisibo Thanks a lot for your effort! 🙂 I will definitively try to fix it and get it into the next version (probably released in December).

  5. Christian Oeing repo owner

    Fixed in 1.21

    The problems were: - A node in the data tree was never removed. Now it is removed after the last listener is removed. - A data context node connector was destroyed incorrect (only value listener was set to null, but the context was not cleared), so it kept a listener for its data node registered.

  6. Christian Oeing repo owner

    @Trisibo I think I found the issue. It fixes the issue in the test project you provided. Would be great if you could test the changed code in your project. I am releasing the next version (1.21) soon and a pre-release test would make me much more comfortable that nothing unexpected broke 🙂

  7. Trinidad Sibajas Bodoque reporter

    I have only tested the update for a bit, but it seems that everything is working fine 🙂 . I’ll let you know if I find any issue with more usage.

  8. Log in to comment