ionic-team / capacitor

Build cross-platform Native Progressive Web Apps for iOS, Android, and the Web ⚡️
https://capacitorjs.com
MIT License
11.76k stars 993 forks source link

[Bug]: Android Bridge Plugin Calls Queued - FileSystem.downloadFile blocking other requests #6861

Open ralphcode opened 1 year ago

ralphcode commented 1 year ago

Bug Report

Capacitor Version

  @capacitor/cli: 5.3.0
  @capacitor/core: 5.3.0
  @capacitor/android: 5.3.0
  @capacitor/ios: 5.3.0

Installed Dependencies:

  @capacitor/cli: 5.3.0
  @capacitor/core: 5.3.0
  @capacitor/android: 5.3.0
  @capacitor/ios: 5.1.1

Platform(s)

Android

Current Behavior

When using Filesystem.downloadFile, it appears to block the plugin thread. Consequently, any other plugin or operation is halted and waits for the download operation to complete. This behavior is particularly pronounced when dealing with large files or during slow network conditions.

Expected Behavior

Filesystem.downloadFile should be non-blocking, allowing other operations to continue. This is essential for us to maintain smooth app responsiveness, especially during large or slow download operations. This affects all requests, so for example Haptics delay and trigger late.

Code Reproduction

Capacitor.Plugins.Filesystem.addListener('progress', (progress) => console.warn(`downloadFile: Progress: `, progress.bytes / progress.contentLength));
console.warn('downloadFile: Starting');
Capacitor.Plugins.Filesystem.downloadFile({ 
    progress: true, recursive: true,
    path: `${(+new Date())}.zip`, 
    url: 'https://sample-videos.com/img/Sample-jpg-image-5mb.jpg', // i.e. 5mb
    directory: 'CACHE' 
}).then(res => console.warn('Download: Complete'), console.error);
console.warn('Request 2: Starting any other request');
Capacitor.Plugins.App.getInfo().then(res => console.warn('Request 2: this is blocked until download completes', typeof res), console.error);

As per logs, Request 2 is not fulfilled until the download completes;

blocking

Other Technical Details

npm --version output: 8.5.0 node --version output: v16.14.2 Android API: v33

Additional Context

I am no expert in Java so not sure what other issues this will introduce however using the ExecutorService for each request does help solve the issue, and edit best concurrent performance is using below;

import java.util.concurrent.Executors;
import java.util.concurrent.ExecutorService;

public class FilesystemPlugin extends Plugin {

    int numberOfCores = Runtime.getRuntime().availableProcessors();
    ExecutorService executorService = Executors.newFixedThreadPool(numberOfCores * 2); // Twice the number of cores

    ...

    @PluginMethod
    public void downloadFile(PluginCall call) {
        executorService.execute(() -> {

            ....

We download and cache images onto their device for offline usage. iOS is fine, handling 15 concurrent downloads. Above can handle ~10 on an older Android device without any major UI impairment. Current code chokes the entire app.

Thanks in advance!

ionitron-bot[bot] commented 1 year ago

This issue needs more information before it can be addressed. In particular, the reporter needs to provide a minimal sample app that demonstrates the issue. If no sample app is provided within 15 days, the issue will be closed. Please see the Contributing Guide for how to create a Sample App. Thanks! Ionitron 💙

ralphcode commented 1 year ago

Example project: https://github.com/ralphcode/capacitor-bug-android-filesystem-blocking/tree/main

Output shows blocking; blocking

ionitron-bot[bot] commented 12 months ago

This issue has been labeled as type: bug. This label is added to issues that that have been reproduced and are being tracked in our internal issue tracker.

jcesarmobile commented 12 months ago

this seems some problem with the bridge and not with downloadFile, on iOS it's working fine, but on Android it looks like the plugin calls are being queued, while it can probably be fixed in filesystem plugin, keeping the issue in Capacitor repository to investigate if it's something we can fix from core so plugin authors don't have to worry about this kind of issues.

tafel commented 10 months ago

Any news about it? I would like to speed up download of multiple files (3 in parallel). Right now, I can only have one download at a time, using Filesystem.downloadFile. Thanks

jn42lm1 commented 9 months ago

+1 for a fix in the core

The Capacitor HTTP plugin uses a workaround similar to what was provided in the issue additional context.

https://github.com/ionic-team/capacitor/blob/main/android/capacitor/src/main/java/com/getcapacitor/plugin/CapacitorHttp.java#L62

In my plugins for now as a workaround I'm wrapping the contents of each plugin function to execute and resolve on it's own thread.

@CapacitorPlugin(name = "MyPlugin")
public class MyPlugin extends Plugin {

  private static final ExecutorService executor = Executors.newCachedThreadPool();

  @PluginMethod()
  public void echo(PluginCall call) {

    executor.submit(() -> {

      try {

        JSObject response = // Do stuff...
        call.resolve(response);

      } catch (Exception e) {

        call.reject(e);
      }

    });
  }
}

This works fine, but is not ideal. While it fixes my own plugins, all the Capacitor official and community plugin calls in my app is still effectively a first-in-first-out queue, e.g. calling something like this is not actually running the plugin calls in parallel. Instead they run one at a time and just resolve together.

const result = await Promise.all([
  App.getInfo(),
  Device.getInfo(),
  Device.getBatteryInfo()
]);
AlexFilenkoDev commented 2 months ago

Any news? Still have a problem Filesystem.downloadFile.

ralphcode commented 1 month ago

FYI: I've updated the example project to Capacitor 6 and issue is still persisting: https://github.com/ralphcode/capacitor-bug-android-filesystem-blocking/tree/main

blocking

mustafa0x commented 1 month ago

Does this plugin have the same problem? https://github.com/AlwaysLoveme/capacitor-plugin-filedownload

(I'm interested in it since it allows downloads to be cancelled.)