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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 [firebase_storage] putFile never resolves when using isolates #9790

Closed
jamesdixon opened this issue Oct 25, 2022 · 11 comments 路 Fixed by #10106
Closed

馃悰 [firebase_storage] putFile never resolves when using isolates #9790

jamesdixon opened this issue Oct 25, 2022 · 11 comments 路 Fixed by #10106
Assignees
Labels
platform: ios Issues / PRs which are specifically for iOS. plugin: storage resolution: fixed A fix has been merged or is pending merge from a PR. type: bug Something isn't working

Comments

@jamesdixon
Copy link

jamesdixon commented Oct 25, 2022

Bug report

Describe the bug
When running any code in an isolate on iOS, Firebase Storage's putFile method no longer resolves and therefore halts any code to be executed after it's run. However, while putFile never resolves, the file is actually uploaded and can be seen in Firebase Storage.

This issue is happening only after adding the flutter_isolate package to our application. We're not running any Firebase Storage code in the isolate -- we only use isolates in a separate image compression process before the upload is triggered.

This is only occurring on iOS and not Android.

This appears to be the same issue that @Wian-TMC opened in #5976 and was automatically closed

All that said, I don't believe that this is a flutter_isolate package issue as this same issue was mentioned many times, using many different packages, in #5976. One of the packages mentioned was audio_service, which also uses isolates.

Steps to reproduce

Steps to reproduce the behavior:

  1. Start with a basic app that uploads to Firebase Storage
  2. Add the flutter_isolate package
  3. Trigger some sort of work in an isolate prior to calling putFile
  4. Call await bucket.putFile(...)

You'll see that the file is uploaded, but it never resolves and therefore any code after putFile is never run.

Expected behavior

Calls to putFile should resolve and continue to the next line when done.

Sample project

https://github.com/SpectoraSoftware/firebase-storage-isolate-issue-sample-app

Please see the README for more details.


Additional context

Add any other context about the problem here.


Flutter doctor

Run flutter doctor and paste the output below:

Click To Expand
Doctor summary (to see all details, run flutter doctor -v):
[鉁揮 Flutter (Channel stable, 3.3.4, on macOS 12.6 21G115 darwin-arm, locale en-US)
[鉁揮 Android toolchain - develop for Android devices (Android SDK version 32.0.0)
[鉁揮 Xcode - develop for iOS and macOS (Xcode 14.0.1)
[鉁揮 Chrome - develop for the web
[鉁揮 Android Studio (version 2021.1)
[鉁揮 VS Code (version 1.72.2)
[鉁揮 Connected device (4 available)
[鉁揮 HTTP Host Availability

Flutter dependencies

Run flutter pub deps -- --style=compact and paste the output below:

Click To Expand
Dart SDK 2.18.2
Flutter SDK 3.3.4
sample 10.0.1+423

dependencies:
- firebase_core 1.24.0 [firebase_core_platform_interface firebase_core_web flutter meta]
- firebase_storage 10.3.11 [firebase_core firebase_core_platform_interface firebase_storage_platform_interface firebase_storage_web flutter]
- flutter 0.0.0 [characters collection material_color_utilities meta vector_math sky_engine]
- flutter_isolate 2.0.3 [flutter uuid]
- path_provider 2.0.11 [flutter path_provider_android path_provider_ios path_provider_linux path_provider_macos path_provider_platform_interface path_provider_windows]

dev dependencies:
- flutter_test 0.0.0 [flutter test_api path fake_async clock stack_trace vector_math async boolean_selector characters collection matcher material_color_utilities meta source_span stream_channel string_scanner term_glyph]
- integration_test 0.0.0 [flutter flutter_driver flutter_test path vm_service archive async boolean_selector characters clock collection crypto fake_async file matcher material_color_utilities meta source_span stack_trace stream_channel string_scanner sync_http term_glyph test_api typed_data vector_math webdriver]

transitive dependencies:
- _flutterfire_internals 1.0.2 [cloud_firestore_platform_interface cloud_firestore_web collection firebase_core firebase_core_platform_interface flutter meta]
- archive 3.3.0 [crypto path]
- async 2.9.0 [collection meta]
- boolean_selector 2.1.0 [source_span string_scanner]
- characters 1.2.1
- clock 1.1.1
- cloud_firestore_platform_interface 5.7.7 [_flutterfire_internals collection firebase_core flutter meta plugin_platform_interface]
- cloud_firestore_web 2.8.10 [_flutterfire_internals cloud_firestore_platform_interface collection firebase_core firebase_core_web flutter flutter_web_plugins js]
- collection 1.16.0
- crypto 3.0.2 [typed_data]
- fake_async 1.3.1 [clock collection]
- ffi 2.0.1
- file 6.1.2 [meta path]
- firebase_core_platform_interface 4.5.1 [collection flutter flutter_test meta plugin_platform_interface]
- firebase_core_web 1.7.3 [firebase_core_platform_interface flutter flutter_web_plugins js meta]
- firebase_storage_platform_interface 4.1.19 [collection firebase_core flutter meta plugin_platform_interface]
- firebase_storage_web 3.3.9 [_flutterfire_internals async firebase_core firebase_core_web firebase_storage_platform_interface flutter flutter_web_plugins http js meta]
- flutter_driver 0.0.0 [file flutter flutter_test fuchsia_remote_debug_protocol path meta vm_service webdriver archive async boolean_selector characters clock collection crypto matcher material_color_utilities platform process source_span stack_trace stream_channel string_scanner sync_http term_glyph test_api typed_data vector_math]
- flutter_web_plugins 0.0.0 [flutter js characters collection material_color_utilities meta vector_math]
- fuchsia_remote_debug_protocol 0.0.0 [process vm_service file meta path platform]
- http 0.13.5 [async http_parser meta path]
- http_parser 4.0.2 [collection source_span string_scanner typed_data]
- js 0.6.4
- matcher 0.12.12 [stack_trace]
- material_color_utilities 0.1.5
- meta 1.8.0
- path 1.8.2
- path_provider_android 2.0.20 [flutter path_provider_platform_interface]
- path_provider_ios 2.0.11 [flutter path_provider_platform_interface]
- path_provider_linux 2.1.7 [ffi flutter path path_provider_platform_interface xdg_directories]
- path_provider_macos 2.0.6 [flutter path_provider_platform_interface]
- path_provider_platform_interface 2.0.5 [flutter platform plugin_platform_interface]
- path_provider_windows 2.1.3 [ffi flutter path path_provider_platform_interface win32]
- platform 3.1.0
- plugin_platform_interface 2.1.3 [meta]
- process 4.2.4 [file path platform]
- sky_engine 0.0.99
- source_span 1.9.0 [collection path term_glyph]
- stack_trace 1.10.0 [path]
- stream_channel 2.1.0 [async]
- string_scanner 1.1.1 [source_span]
- sync_http 0.3.1
- term_glyph 1.2.1
- test_api 0.4.12 [async boolean_selector collection meta source_span stack_trace stream_channel string_scanner term_glyph matcher]
- typed_data 1.3.1 [collection]
- uuid 3.0.6 [crypto]
- vector_math 2.1.2
- vm_service 9.0.0
- webdriver 3.0.0 [archive matcher path stack_trace sync_http]
- win32 3.0.1 [ffi]
- xdg_directories 0.2.0+2 [meta path process]

@jamesdixon jamesdixon added Needs Attention This issue needs maintainer attention. type: bug Something isn't working labels Oct 25, 2022
@jnarowski
Copy link

+1 to this issue. We're blocked from using Isolates until we figure this out.

@JavierPerezLavadie
Copy link

@jamesdixon
Copy link
Author

@JavierPerezLavadie were not on the 10.x sdk.

@danagbemava-nc danagbemava-nc added the triage Issue is currently being triaged. label Oct 26, 2022
@danagbemava-nc
Copy link

I can reproduce the issue using the code sample in https://github.com/SpectoraSoftware/firebase-storage-isolate-issue-sample-app.

Labeling for further investigation.

@danagbemava-nc danagbemava-nc added plugin: storage platform: ios Issues / PRs which are specifically for iOS. and removed Needs Attention This issue needs maintainer attention. triage Issue is currently being triaged. labels Oct 26, 2022
@jamesdixon
Copy link
Author

A heads up that I just tried all of the latest Firebase dependencies that use SDK 10 under the hood and I am seeing the exact same issue.

@jamesdixon
Copy link
Author

@danagbemava-nc do you have any sense of what the priority would be for this issue?

I realize there are probably many things in the queue to be looked at, so I promise that I'm not being pushy 馃槃

This is a major blocker for us so we're considering whether or not we wait for this to be fixed in the underlying SDK or if we need to proceed with moving to S3 for our storage needs.

Thank you!

@GabrielAraujo
Copy link

Hello Folks, I did a bit of investigation on my end and was able to find the issue.

First, when we use the flutter_isolate library, whenever you spawn a new isolate it will look all flutter plugins that were registered and will register all of then again on the engine for the isolate. Code reference

Because of that the firebase_storage plugin registration function will be called again but the channel for the shared storage instance will be overridden with the new channel created after the isolate spawning. Because of those actions, when it is time for the method channel Task#onSuccess to be called the channel reference will be null (because after the isolate is killed the cleanup function is called setting the channel to nil) or with a different channel reference than the one originally created, so the response hangs.

I've tested the solution bellow and it seems to work:

+ (void)registerWithRegistrar:(NSObject<FlutterPluginRegistrar> *)registrar {
  FlutterMethodChannel *channel =
      [FlutterMethodChannel methodChannelWithName:kFLTFirebaseStorageChannelName
                                  binaryMessenger:[registrar messenger]];

  FLTFirebaseStoragePlugin *instance = [FLTFirebaseStoragePlugin sharedInstance];
    if (instance.channel == nil) {
        instance.channel = channel;
      #if TARGET_OS_OSX
        // TODO(Salakar): Publish does not exist on MacOS version of FlutterPluginRegistrar.
      #else
        [registrar publish:instance];
      #endif
        [registrar addMethodCallDelegate:instance channel:channel];
    }
}

It would be nice for someone on the flutterfire team to take a look so I can open a MR or so we can discuss better solutions.

@GabrielAraujo
Copy link

If anyone wants to test out, I've forked the flutterfire repo and created a branch with this fix.

Just add the dependency override at the pubspec.yaml as bellow:

dependency_overrides:
  firebase_storage:
    git:
      url: https://github.com/SpectoraSoftware/flutterfire
      ref: 9.6.0-isolate-fix
      path: packages/firebase_storage/firebase_storage

@GabrielAraujo
Copy link

Hello! Just a quick update with more info for next steps:

By using the flutter_isolate dependency, we can declare which plugins we would like to register for the isolate being spawned.
This way we prevent the firebase plugins to be registered again but this also means we will not be able to use the firebase sdks at the isolate execution.

AppDelegate.swif

// Defines a custom plugin registrant, to be used specifically together with FlutterIsolatePlugin
@objc(IsolatePluginRegistrant) class IsolatePluginRegistrant: NSObject {
    @objc static func register(withRegistry registry: FlutterPluginRegistry) {
        // Register channels for Flutter Isolate
        ImageCompressPlugin.register(with: registry.registrar(forPlugin: "ImageCompressPlugin")!)
        FLTPathProviderPlugin.register(with: registry.registrar(forPlugin: "FLTPathProviderPlugin")!)
    }
}

@UIApplicationMain
@objc class AppDelegate: FlutterAppDelegate {
  override func application(
    _ application: UIApplication,
    didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?
  ) -> Bool {
    FlutterIsolatePlugin.isolatePluginRegistrantClassName = "IsolatePluginRegistrant"
      
    GeneratedPluginRegistrant.register(with: self)

    if #available(iOS 10.0, *) {
      UNUserNotificationCenter.current().delegate = self as? UNUserNotificationCenterDelegate
    }

    return super.application(application, didFinishLaunchingWithOptions: launchOptions)
  }
}

@Lyokone Lyokone self-assigned this Nov 2, 2022
@Lyokone
Copy link
Contributor

Lyokone commented Nov 2, 2022

Hello @GabrielAraujo , currently FlutterFire might not work with isolates, I'll check your provided code, thanks :)

@siddharthadevops
Copy link

This is a big issue since many apps need isolates to perform heavy tasks. Now I am in the final stage of my project, optimizing some tasks and I found this problem which is blocking. Please, consider this important.

@danagbemava-nc danagbemava-nc added the resolution: fixed A fix has been merged or is pending merge from a PR. label Dec 29, 2022
@firebase firebase locked and limited conversation to collaborators Jan 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
platform: ios Issues / PRs which are specifically for iOS. plugin: storage resolution: fixed A fix has been merged or is pending merge from a PR. type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants