[Bug]: When using TransformAsync, then Bind sometimes does not remove item from final collection

[tomasfil]

Describe the bug 🐞

With TransformAsync and Bind behind that, when item is rapidly added and then removed, then once SourceCache is empty, for some reason and only sometimes there are left items in the destination ReadOnlyObservableCollection.

Step to reproduce

I have created breaking Unit test in repro repo branch TransformAsyncWithBind

Reproduction repository

https://github.com/tomasfil/DynamicData/tree/TransformAsyncWithBind

Expected behavior

This should not happen...

Screenshots 🖼️

No response

IDE

No response

Operating system

No response

Version

No response

Device

No response

DynamicData Version

No response

Additional information ℹ️

No response

[tomasfil]

I did some debugging, check person 61 for instance, remove message got faster than the add to arrive in Bind

TransformAsync Name1. 1
Add Bind Name1. 1
Remove Bind Name1. 1
TransformAsync Name2. 2
Add Bind Name2. 2
Remove Bind Name2. 2
TransformAsync Name3. 3
Add Bind Name3. 3
Remove Bind Name3. 3
TransformAsync Name4. 4
Add Bind Name4. 4
Remove Bind Name4. 4
TransformAsync Name5. 5
Add Bind Name5. 5
Remove Bind Name5. 5
TransformAsync Name6. 6
Add Bind Name6. 6
Remove Bind Name6. 6
TransformAsync Name7. 7
Add Bind Name7. 7
Remove Bind Name7. 7
TransformAsync Name8. 8
Add Bind Name8. 8
Remove Bind Name8. 8
TransformAsync Name9. 9
Add Bind Name9. 9
Remove Bind Name9. 9
TransformAsync Name10. 10
Add Bind Name10. 10
Remove Bind Name10. 10
TransformAsync Name11. 11
Add Bind Name11. 11
Remove Bind Name11. 11
TransformAsync Name12. 12
Add Bind Name12. 12
Remove Bind Name12. 12
TransformAsync Name13. 13
Add Bind Name13. 13
Remove Bind Name13. 13
TransformAsync Name14. 14
Add Bind Name14. 14
Remove Bind Name14. 14
TransformAsync Name15. 15
Add Bind Name15. 15
Remove Bind Name15. 15
TransformAsync Name16. 16
Add Bind Name16. 16
Remove Bind Name16. 16
TransformAsync Name17. 17
Add Bind Name17. 17
Remove Bind Name17. 17
TransformAsync Name18. 18
Add Bind Name18. 18
Remove Bind Name18. 18
TransformAsync Name19. 19
Add Bind Name19. 19
Remove Bind Name19. 19
TransformAsync Name20. 20
Add Bind Name20. 20
Remove Bind Name20. 20
TransformAsync Name21. 21
Add Bind Name21. 21
Remove Bind Name21. 21
TransformAsync Name22. 22
Add Bind Name22. 22
Remove Bind Name22. 22
TransformAsync Name23. 23
Add Bind Name23. 23
Remove Bind Name23. 23
TransformAsync Name24. 24
Add Bind Name24. 24
Remove Bind Name24. 24
TransformAsync Name25. 25
Add Bind Name25. 25
Remove Bind Name25. 25
TransformAsync Name26. 26
Add Bind Name26. 26
Remove Bind Name26. 26
TransformAsync Name27. 27
Add Bind Name27. 27
Remove Bind Name27. 27
TransformAsync Name28. 28
Add Bind Name28. 28
Remove Bind Name28. 28
TransformAsync Name29. 29
Add Bind Name29. 29
Remove Bind Name29. 29
TransformAsync Name30. 30
Add Bind Name30. 30
Remove Bind Name30. 30
TransformAsync Name31. 31
Add Bind Name31. 31
Remove Bind Name31. 31
TransformAsync Name32. 32
Add Bind Name32. 32
Remove Bind Name32. 32
TransformAsync Name33. 33
Add Bind Name33. 33
Remove Bind Name33. 33
TransformAsync Name34. 34
Add Bind Name34. 34
Remove Bind Name34. 34
TransformAsync Name35. 35
Add Bind Name35. 35
Remove Bind Name35. 35
TransformAsync Name36. 36
Add Bind Name36. 36
Remove Bind Name36. 36
TransformAsync Name37. 37
Add Bind Name37. 37
Remove Bind Name37. 37
TransformAsync Name38. 38
Add Bind Name38. 38
Remove Bind Name38. 38
TransformAsync Name39. 39
Add Bind Name39. 39
Remove Bind Name39. 39
TransformAsync Name40. 40
Add Bind Name40. 40
Remove Bind Name40. 40
TransformAsync Name41. 41
Add Bind Name41. 41
Remove Bind Name41. 41
TransformAsync Name42. 42
Add Bind Name42. 42
Remove Bind Name42. 42
TransformAsync Name43. 43
Add Bind Name43. 43
Remove Bind Name43. 43
TransformAsync Name44. 44
Add Bind Name44. 44
Remove Bind Name44. 44
TransformAsync Name45. 45
Add Bind Name45. 45
Remove Bind Name45. 45
TransformAsync Name46. 46
Add Bind Name46. 46
Remove Bind Name46. 46
TransformAsync Name47. 47
Add Bind Name47. 47
Remove Bind Name47. 47
TransformAsync Name48. 48
Add Bind Name48. 48
Remove Bind Name48. 48
TransformAsync Name49. 49
Add Bind Name49. 49
Remove Bind Name49. 49
TransformAsync Name50. 50
Add Bind Name50. 50
Remove Bind Name50. 50
TransformAsync Name51. 51
Add Bind Name51. 51
Remove Bind Name51. 51
TransformAsync Name52. 52
Add Bind Name52. 52
Remove Bind Name52. 52
TransformAsync Name53. 53
Add Bind Name53. 53
Remove Bind Name53. 53
TransformAsync Name54. 54
Add Bind Name54. 54
Remove Bind Name54. 54
TransformAsync Name55. 55
Add Bind Name55. 55
Remove Bind Name55. 55
TransformAsync Name56. 56
Add Bind Name56. 56
Remove Bind Name56. 56
TransformAsync Name57. 57
Add Bind Name57. 57
Remove Bind Name57. 57
TransformAsync Name58. 58
Add Bind Name58. 58
Remove Bind Name58. 58
TransformAsync Name59. 59
Add Bind Name59. 59
Remove Bind Name59. 59
TransformAsync Name60. 60
Add Bind Name60. 60
Remove Bind Name60. 60
TransformAsync Name61. 61
Remove Bind Name61. 61
Add Bind Name61. 61
TransformAsync Name62. 62
Add Bind Name62. 62
Remove Bind Name62. 62
TransformAsync Name63. 63
Add Bind Name63. 63
Remove Bind Name63. 63

[tomasfil]

Seems like very weird scheduling issue

[RolandPheasant]

😱 😱 😱

TransformAsync has never, ever been right!

[tomasfil]

wdym? It worked well, but I managed to optimize my code so well, it became too fast and I discovered this issue :D :D

[tomasfil]

Okay... I cannot figure it out :D

[RolandPheasant]

"wdym"

Over the years, there's been loads of issues with TransformAsync

[tomasfil]

From my debugging the issue appears in .Bind, the remove reason arrives before add

[RolandPheasant]

the remove reason arrives before add

That'll be the sympton, not the cause. The cause is before bind

[tomasfil]

I found it!!!!

I did test where is the earliest point of swap with:

            ChangeReason changeReasonBefore = ChangeReason.Update;
            ChangeReason changeReasonAfter = ChangeReason.Update;
            
            var transformer = _source.SelectMany(async changes =>
            {
                var change = changes.First();
                try
                {
                    await asyncLock.WaitAsync();
                    if (change.Reason == ChangeReason.Remove && changeReasonBefore == ChangeReason.Remove)
                    {
                          ///// EVERYTHING IS FINE
                    }
                    changeReasonBefore = change.Reason;
                    return await DoTransform(cache, changes).ConfigureAwait(false);

                }
                finally
                {
                    asyncLock.Release();
                    if (change.Reason == ChangeReason.Remove && changeReasonAfter == ChangeReason.Remove)
                    {
                                  ///////// THERE IS THE POINT THAT ORDER GETS MESSED UP
                    }
                    changeReasonAfter = change.Reason;
                }
            });

For some reason the race condition is in the finnaly, like what the hell? .NET hello?

Doing this fixes it, but it needs to be polished

            var transformer = _source.SelectMany(async changes =>
            {
                    await asyncLock.WaitAsync();
                    return await DoTransform(cache, changes).ConfigureAwait(false);
            })
            .Do(_=> asyncLock.Release());

[tomasfil]

Maybe this like this with finnaly to prevent dead lock?

            var transformer = _source.SelectMany(async changes =>
            {
                await asyncLock.WaitAsync();
                return await DoTransform(cache, changes).ConfigureAwait(false);
            })
            .Do(_ => asyncLock.Release())
            .Finally(_ => asyncLock.Release());

But my head cannot wrap around how the finally can screw thing up like that, or maybe the .Release?

---EDIT, no go, this breaks other unit tests

[tomasfil]

I am trying to reproduce the issue without the cache, but I am unable to...

I am still baffled how that happens in the TransformAsync

    public async Task FinnalyRace()
    {
        var people = Enumerable.Range(0,1000).Select(i=> new Person(i.ToString(),i)).ToList();
        var semaphore = new SemaphoreSlim(1,1);
        var sub = new Subject<Person>();
        var lastPerson = new Person("-1",-1);

        sub.SelectMany(async person =>
        {
            await semaphore.WaitAsync();
            try
            {
                await Task.Yield();
                await Task.Delay(10).ConfigureAwait(false);
                return person;
            }
            finally
            {
                semaphore.Release();
                if (lastPerson.Age > person.Age)
                {
                    //// RACE
                }
                lastPerson = person;

            }
        }).Subscribe();

        foreach (var person in people)
        {
            sub.OnNext(person);
        }
    }

[RolandPheasant]

Thanks for raising the PR. It is baffling and I have been scratching my head on this one. Let's sit on it a little and try to work it out at a deeper level. Hopefully we can gain a better understanding but worst case scenario we could just merge it anyway.