xamarin / Essentials

Xamarin.Essentials is no longer supported. Migrate your apps to .NET MAUI, which includes Maui.Essentials.
https://aka.ms/xamarin-upgrade
Other
1.52k stars 505 forks source link

[Bug] [UWP] App crashes when picking more than 1,000 files #2061

Closed DanielGilbert closed 1 year ago

DanielGilbert commented 1 year ago

Description

When picking more than 1000 files on UWP at once using PickMultipleAsync() or when picking 1000 different files in a row with PickAsync(), the application will crash with a System.Exception, stating that:

The maximum number of items in the access list has been reached. An item must be removed before another item is added.

This is because the platform specific implementation for UWP is adding every file to StorageApplicationPermissions.FutureAccessList, which has a maximum storage capacity of 1000 files. This list is kept between app restarts and presumably also updates, and should only be managed by the app itself.

The issue surfaced for us after many months of usage, were a single file was added on a day-by-day base.

Steps to Reproduce

  1. Call PickMultipleAsync for UWP
  2. Pick more than 1000 files (or less, depending on how many have been picked before)
  3. Click "Open" in the file dialog.

-- or --

Checkout the attached archive, which has a sample UWP app and 1001 random files. Simply click on PickFiles and select all files from the Sample Data folder. Then press "Open" and wait for the app to crash.

Expected Behavior

The app should continue to work as expected.

Actual Behavior

The app takes a long time to load all files and will finally crash with a System.Exception, after the maximum amount of items has been reached.

Basic Information

Reproduction Link

FilePickerUwpTest.zip

DanielGilbert commented 1 year ago

In case you need help implementing a fix on this item (PR or anything), I'm happy to sign a CLA and contribute. :)

jfversluis commented 1 year ago

@DanielGilbert your best shot at getting this fixed is probably if you indeed contribute. So if you're willing and able, you might want to give that a shot. Thanks!

DanielGilbert commented 1 year ago

@DanielGilbert your best shot at getting this fixed is probably if you indeed contribute. So if you're willing and able, you might want to give that a shot. Thanks!

Will do! Didn't want to start with a PR without asking beforehand. :)

MitchBomcanhao commented 1 year ago

if you simply remove adding the file to the future access list, you might be causing other stuff to break (this line of code wouldn't be there for no reason, and in the past we've had to use it on our own windows file picker because it'd not work correctly without it - eg it'll possibly fail as the app will not have permission to access the files you just picked.

I'm not sure how i'd handle the "pick over 1000 files" scenario, for a smaller number I'd just clear the older files in the future access list - they're likely to be old enough to have been handled by previous picking actions and therefore will no longer need to be in this list. I'd also consider clearing this list on application launch.

to be clear - my use case is:

MitchBomcanhao commented 1 year ago

what we potentially need is a way to copy the files into the app's storage as part of the file picker code, which would allow the file picker to be self contained and then we could immediately remove the files from the future access list.

eg

if the user picks more than 1000 files, we can potentially do this processing in chunks, to guarantee that you'll never go over the 1000 files limit?

DanielGilbert commented 1 year ago

Hadn't had time to create the PR yet, sorry.

what we potentially need is a way to copy the files into the app's storage as part of the file picker code, which would allow the file picker to be self contained and then we could immediately remove the files from the future access list.

I'm a strong opponent on this one. The filepicker implementation already returns the StorageFile object. As long as you do not dispose this object, you can access the underlying file, and copy it everywhere you like.

I highly recommend re-reading the linked documentation at the beginning of my post. You should store this token, and use GetFileAsync or GetFolderAsync after you've disposed the StorageFile instance, so that you can get access again. You won't automatically get access to this file in this path, just because it's in this list. :) The Add function that is called in the linked code has a return value which is discarded by this implementation. The return value is the token you need to get access again to this file.

The most important part is, that the app should handle this list exclusively, so it can be cleaned up by the app. In our app, we are explicitly clearing the list at the moment right after the file gets added as a work around. If we would use this list for our app, it would be a real pain to filter.

MitchBomcanhao commented 1 year ago

ok, will do