Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Race condition between page fetch and item list update causing item duplication #266

Closed
Ahmed-Omar-Hommir opened this issue May 29, 2023 · 9 comments

Comments

@Ahmed-Omar-Hommir
Copy link

Ahmed-Omar-Hommir commented May 29, 2023

Hi,

This issue seems to be a race condition that occurs when a new page is being fetched (via addPageRequestListener) and the item list is being updated (via UpdateFavorite).

Here's a brief overview of the situation:

`
pagingController.addPageRequestListener((page) async {
add(GetRestaurants(page));
});

on((event, emit) {
final itemList = pagingController.itemList;
if (itemList == null || itemList.isEmpty) return;

ListRestaurantDM restaurants = ListRestaurantDM(restaurants: itemList);

restaurants = restaurants.updateFavorite(
restaurantId: event.restaurantId,
isFavorite: event.isFavorite,
);
//When updated itemList and the pagingControllerState is "ongoing" the addPageRequestListener listens again for the same page.
pagingController.itemList = restaurants.restaurants;
});
`

The problem occurs when GetRestaurants is called to fetch a new page (which means the state is ongoing), and UpdateFavorite is called concurrently to update the itemList of the pagingController. This results in the same page being called multiple times, leading to item duplication in the list.

This issue only occurs when a page fetch is in progress and the item list is updated simultaneously.

Do you have any recommendations on how to resolve this, or is this something that needs to be addressed within the package itself?

Thank you for your time and assistance.

@Ahmed-Omar-Hommir
Copy link
Author

Widget _buildListItemWidget(
BuildContext context,
int index,
List itemList,
) {
if (!_hasRequestedNextPage) {
final newPageRequestTriggerIndex =
max(0, _itemCount - _invisibleItemsThreshold);

  final isBuildingTriggerIndexItem = index == newPageRequestTriggerIndex;

  if (_hasNextPage && isBuildingTriggerIndexItem) {
    // Schedules the request for the end of this frame.
    WidgetsBinding.instance.addPostFrameCallback((_) {
      _pagingController.notifyPageRequestListeners(_nextKey!);
    });
    _hasRequestedNextPage = true;
  }
}

final item = itemList[index];
return _builderDelegate.itemBuilder(context, item, index);

}

How to prevent the method from calling notifyPageRequestListeners when the paging is ongoing?
the notifyPageRequestListeners is already called (in progress) while building the widget.

@daoxve
Copy link

daoxve commented Jun 1, 2023

Hi, were you able to resolve this?

@hvermaaajtak
Copy link

I'm also facing the same issue.

@faisalansari0367
Copy link

Please create gist so that we can replicate the issue to fix it.

@daoxve
Copy link

daoxve commented Jun 9, 2023

I have a page with multiple pagingControllers, each fetching data for it's respective tab, and each with infinite items. I was able to workaround this issue by adding a debouncer to each of the pagination methods passed to addPageRequestListener. I just needed to add a short delay and pass the same unique tag to all the pagination methods, so that no two requests can ever be triggered simultaneously. This ultimately resolved the race condition, and so the duplication for me. Code looks like so:

_tabPagingController.addPageRequestListener((pageKey) {
      debounce(
        'uniqueTag',
        const Duration(milliseconds: 500),
        () => _paginate(
          pageKey: pageKey,
          pagingController: _tabPagingController,
          futureToRun: futureToRunForSpecificTab(currentPage: pageKey),
        ),
      );
    });

@EdsonBueno
Copy link
Owner

We need a working example. @faisalansari0367 asked but no response was given from the reporters. Closing the issue for now, but feel free to reopen.

@Ahmed-Omar-Hommir
Copy link
Author

Ahmed-Omar-Hommir commented Sep 16, 2023

Hi, I sincerely apologize for the delay in my response. @faisalansari0367 @EdsonBueno

Issue Description

While fetching the next page, if the user updates an item's state (for example, changing an item's 'favorite' status), the list state is updated through pagingController.itemList = updatedItems;. This also triggers another request for the same page.

REC-20230916174545.mp4

@EdsonBueno The issue also exists in the WonderWords app.

Note: I added a delay to my request to simulate scenarios, which does not mean that the problem rarely occurs. This has happened to me so many times, that even with the QA testing team, I had to copy and modify the package to implement a quick fix.

My Solution

The ongoing state is not enough. I added a new state called fetchingNextPage. This state will be used to indicate whether the client is in the process of fetching the next page. While the state is fetchingNextPage, any event that triggers a next page fetch will be ignored. This will prevent multiple requests for the same page number in the scenarios you described.

Before:
flow-diagram (1)

After:
Group 27699

@Sovann72
Copy link

Sovann72 commented Sep 25, 2023

I have a page with multiple pagingControllers, each fetching data for it's respective tab, and each with infinite items. I was able to workaround this issue by adding a debouncer to each of the pagination methods passed to addPageRequestListener. I just needed to add a short delay and pass the same unique tag to all the pagination methods, so that no two requests can ever be triggered simultaneously. This ultimately resolved the race condition, and so the duplication for me. Code looks like so:

_tabPagingController.addPageRequestListener((pageKey) {
      debounce(
        'uniqueTag',
        const Duration(milliseconds: 500),
        () => _paginate(
          pageKey: pageKey,
          pagingController: _tabPagingController,
          futureToRun: futureToRunForSpecificTab(currentPage: pageKey),
        ),
      );
    });

it does not works on my case.
here is my scenario: I have 3 tabs on infinite scrolling items. each has a refresh indicator. onRefresh it will call pagingController.refresh. however every time i switch back to other page and come back to previous page and refresh it will fetch repeatedly the same page. example:
tab1 has 1 page of 2 items.

go to tab2
go back to tab1
refresh tab1
tab1 now has 4 items. 2 of which are the same.
go to tab2 again
go back to tab1
refresh tab1
tab1 now has 6 items. 3 of them are the same items

idk when did the pagingController called another notifyPageRequestListener after I already called pagingController.refresh. also why does it happen to tabbar? can anyone explain to me here.

@Suman085
Copy link

I have the same exact issue @Sovann72 , @Ahmed-Omar-Hommir . Were you guys able to find a workaround?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants