bkonyi / FlutterGeofencing

Rough work for Flutter geofencing plugin
BSD 3-Clause "New" or "Revised" License
338 stars 220 forks source link

The example Application.kt registration may cause crashes when used with other registered plugins #3

Closed dustin-graham closed 4 years ago

dustin-graham commented 5 years ago

@bkonyi Firstly, thank you for kindly offering this plugin code and writing the blog post. Awesome work!

Today I decided to try and integrate this plugin as an experiment to the app I'm building as part of a hack day at work. Everything went great but I did bump into a few issues that I had to work around and I wanted to report these troubles with hopes we can find the best way around them. The problem has to do with how the geofencing plugin is registered via the custom Application class..

https://github.com/bkonyi/FlutterGeofencing/blob/d6c88bb37ce513a782cbf26cbcb2b2715202b63f/example/android/app/src/main/kotlin/io/flutter/plugins/geofencingexample/Application.kt#L16

When the service starts and uses this method to register, and as a side effect, the GeneratedPluginRegistrant runs through all the other plugins the host application may have installed and registers them as well. In my case I had several that expected to receive an Activity in the registrar. But when registering in this background context, there was no Activity delivered and my app crashed. As a temporary work around I decided not to use the GeneratedPluginRegistrant and instead jut copied lines out of that class and stuck them right in my Application.kt. This ended up being safe and effective since this code path is specifically for the geofencing plugin to work. Here's a snippet:

class Application : FlutterApplication(), PluginRegistry.PluginRegistrantCallback {
    override fun onCreate() {
        super.onCreate();
        GeofencingService.setPluginRegistrant(this);
    }

    override fun registerWith(registry: PluginRegistry) {
        if (alreadyRegisteredWith(registry)) {
            return
        }
        GeofencingPlugin.registerWith(registry.registrarFor("io.flutter.plugins.geofencing.GeofencingPlugin"))
        FlutterLocalNotificationsPlugin.registerWith(registry.registrarFor("com.dexterous.flutterlocalnotifications.FlutterLocalNotificationsPlugin"))
    }

    private fun alreadyRegisteredWith(registry: PluginRegistry): Boolean {
        val key = GeneratedPluginRegistrant::class.java.canonicalName
        if (registry.hasPlugin(key)) {
            return true
        }
        registry.registrarFor(key)
        return false
    }
}

Notice here that I also included the Flutter Local Notifications plugin. My implementation needed to show a local notification when the user interacts with the geofence. The app crashed on my first attempt to call the notification plugin from the background isolate and then it occurred to me that from that isolate context, the local notifications plugin was never registered, so it needed to tag along with the geofencing plugin. I'm basically duplicating what the GeneratedPluginRegistrant does but only including the plugins I need and which I know will work without an Activity context.

This is obviously pretty messy and fragile and I'm feeling like this is maybe an are that needs a little more thought towards an official solution. Perhaps a way to have a GeneratedBackgroundPluginRegistrant class generated for plugins that can support execution in the background without an Activity context? Just a thought.

Basically there's two challenges:

  1. How to safely register plugins in the background that play nice with plugins that assume they will be registered in the foreground?
  2. How best to utilize other plugins from our background isolate.

In my solution I'm just manually registering the necessary plugins, but I can see how this might not be very friendly to work with. Just wanted to send you some of my notes with hopes that perhaps you have some ideas to improve this situation.

Thanks! Dustin

bkonyi commented 4 years ago

Sorry for the delay and thank you for the detailed write up! I wasn't subscribed to notifications on this repo for some reason and missed all the filed issues...

Anyway, I know this is way too late for your hackathon but this is a known limitation of how Flutter currently handles plugin registration. This plugin, along with the android_alarm_manager have methods to provide access to the GeneratedPluginRegistrant after startup which is used to register all plugins with the newly spawned background isolate. Unfortunately, there's no way to specify which plugins should be registered with the isolate which can cause problems if you also have plugins that require the application UI to be present (e.g., an Android Activity or something similar). In that situation, the application will likely crash if a background event occurs while the application is backgrounded (e.g., entering a geofence) depending on what state the newly registered plugins try to initialize. As you've discovered, the best way around this currently is to manually register plugins that you need while running in the background.

However, the good news is that, at least for Android, this limitation should be resolved in an upcoming Flutter release (hopefully before EOY) once we move over to using a new API which can handle these cases.