f-miyu / Plugin.CloudFirestore

MIT License
121 stars 44 forks source link

System.ObjectDisposedException when using IQuerySnapshot.Documents.ToList() #84

Closed mrobraven closed 3 years ago

mrobraven commented 3 years ago

Okay, so I began with this script:

foreach (IDocumentSnapshot row in postArray.Documents)
{

}

Which only worked on the odd occasion or when I disabled other awaited asynchronous calls within the loop. I saw in issue #63 a possible fix using .ToList() which worked a treat and greatly reduced the amount of times the loop throws an error:

foreach (IDocumentSnapshot row in postArray.Documents.ToList())
{

}

However, the new error I am receiving is different and happens when awaited async calls take a long time to complete within the loop (IE: the ones that download image and other file data from firebase): System.ObjectDisposedException

Cannot access a disposed object.
Object name: 'Firebase.Firestore.QueryDocumentSnapshot'

I am building with Xamarin. The postArray variable is a member of the page class:

public partial class ProfileView : ContentPage
{
        IQuerySnapshot postArray;
}

And is set through a method called GetPostsAsync:

async Task GetPostsAsync(IDocumentSnapshot startPage = null)
{
            postArray = await postHandle.ListUserPosts(userID, startPage);

            if(startPage == null)
            {
                recentPostArray = postArray;
            }
}

The method ListUserPosts simply builds a Firestore query and returns the result of the .GetAsync() method. The resulting postArray is then accessed from a function called DisplayPosts() which includes the above loop

I have no idea why the garbage collecting service is disposing of the IDocumentSnapshot before the loop has finished. I have had similar errors before whilst developing which have all ironed themselves out some way or another but this one has me completely stuck. I fear that when rolled out, the application will crash if users have too many posts to display on their pages.

angelru commented 3 years ago

I have also been with similar errors: https://github.com/f-miyu/Plugin.CloudFirestore/issues/56

I think it has to do with the collection being accessed from different threads, I have "fixed" it by creating auxiliary list and not using an await within a foreach.

create a list of tasks in your loop and then when done use Task.WhenAll

mrobraven commented 3 years ago

Hi,

Thanks @angelru ! I shall try and implement this.

Incase anyone else is interested in this issue, my current (but nowhere near sufficient) fix was to reduce the frequency of the garbage collection by adding an environment variable to the app that triggers the compiler to use the old version of the garbage collector:

This line: MONO_GC_PARAMS=bridge-implementation=old

Should be added to a text file anywhere in the android project. I have named mine environment.txt marked with build action AndroidEnvironment.

I shall close this issue for now as it is my issue and not an issue with the plugin but I will keep you updated on whether your suggestion works out in my case @angelru

angelru commented 3 years ago

Also do not use an await inside a foreach, it is inefficient.

Good!

List<Task> tasks = new List<Task>();
foreach (int i=0;i<20;i++)
{
  tasks.Add(GetAsync(i));
}

await Task.WhenAll(tasks);

here's something akin to the GC thing you said, and it improved the Scroll in CollectionView quite a bit with a complex xaml layout.

https://codetraveler.io/2020/07/12/improving-collectionview-scrolling/

Regards