React-native: Reload (from dev menu) should call onHostDestroy or any other event to detect Reload

[mauron85]

Thank you guys for amazing work

Issue Description

I'm developing react-native-background-geolocation. While working on this issue, I've identified potential problem within my module logic and Reload (from dev menu) functionality.

In my module I'm creating LocationService similar to android MessengerService from android examples.
In BackgroundGeolocationModule I'm binding to this service (and starting it) on user request (from js). Module should unbind from service on onHostDestroy (LifecycleEventListener) method (this part is not in my repo yet).

However it seems Reload doesn't call onHostDestroy or any other event to detect Reload event. As result client (module) client will be binding to service twice, which leads to java.lang.IllegalStateException: "This message is already in use" and app crash.

AndroidRuntime  D  Shutting down VM
                E  FATAL EXCEPTION: main
                E  Process: com.marianhello.RNBGExample, PID: 11937
                E  Theme: themes:{default=overlay:com.resurrectionremix.pitchblack, fontPkg:com.resurrectionremix.pitchblack, com.android.systemui=overlay:com.resurrectio
                   nremix.pitchblack, com.android.systemui.navbar=overlay:com.resurrectionremix.pitchblack}
                E  java.lang.IllegalStateException: { when=0 what=4 target=com.marianhello.react.BackgroundGeolocationModule$IncomingHandler } This message is already in 
                   use.
                E      at android.os.MessageQueue.enqueueMessage(MessageQueue.java:538)
                E      at android.os.Handler.enqueueMessage(Handler.java:631)
                E      at android.os.Handler.sendMessageAtTime(Handler.java:600)
                E      at android.os.Handler.sendMessageDelayed(Handler.java:570)
                E      at android.os.Handler.sendMessage(Handler.java:507)
                E      at android.os.Handler$MessengerImpl.send(Handler.java:721)
                E      at android.os.Messenger.send(Messenger.java:57)
                E      at com.marianhello.bgloc.LocationService.sendClientMessage(LocationService.java:369)
                E      at com.marianhello.bgloc.LocationService.handleLocation(LocationService.java:348)
                E      at com.marianhello.bgloc.AbstractLocationProvider.handleLocation(AbstractLocationProvider.java:81)
                E      at com.tenforwardconsulting.bgloc.DistanceFilterLocationProvider.onLocationChanged(DistanceFilterLocationProvider.java:318)
                E      at android.location.LocationManager$ListenerTransport._handleMessage(LocationManager.java:285)
                E      at android.location.LocationManager$ListenerTransport.-wrap0(LocationManager.java)
                E      at android.location.LocationManager$ListenerTransport$1.handleMessage(LocationManager.java:230)
                E      at android.os.Handler.dispatchMessage(Handler.java:102)
                E      at android.os.Looper.loop(Looper.java:148)
                E      at android.app.ActivityThread.main(ActivityThread.java:5461)
                E      at java.lang.reflect.Method.invoke(Native Method)
                E      at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
                E      at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)
 unknown:React  W  Calling JS function after bridge has been destroyed.

Steps to Reproduce / Code Snippets

1. Create android Service similar to MessengerService
2. In ReactContextBaseJavaModule bind to it.

...
void doBindService() {
    // Establish a connection with the service.  We use an explicit
    // class name because there is no reason to be able to let other
    // applications replace our component.
    if (mIsBound) { return; } //member variable to prevent client to bind multiple times, but doesn't help when Reload is done (Module is reinstantiated so it is false again)

    mMessenger = new Messenger(new IncomingHandler());

    final Activity currentActivity = this.getCurrentActivity();
    Intent locationServiceIntent = new Intent(currentActivity, LocationService.class);
    locationServiceIntent.putExtra("config", mConfig);
    currentActivity.bindService(locationServiceIntent, mConnection, Context.BIND_IMPORTANT);
  }
...

1. In some point. Do app Reload from dev menu
2. Send some IPC Message from Service to client.
3. App will crash, because client was registered twice, so same Message was sent twice to same client (which is illegal action in Android)
   

   Expected Results

Event that is sent before react-native will do Reload, so we can unbind from service.

Additional Information

Addtional description also here:
http://stackoverflow.com/questions/39354236/android-how-to-prevent-binding-to-remote-messenger-service-twice-to-prevent-java?noredirect=1#comment66039454_39354236

• React Native version: 0.29
• Platform(s) (iOS, Android, or both?): Android
• Operating System (macOS, Linux, or Windows?): all

[charpeni]

@facebook-github-bot label Icebox

[charpeni]

Hi there! This issue is being closed because it has been inactive for a while.

But don't worry, it will live on with ProductPains! Check out its new home: https://productpains.com/post/react-native/reload-from-dev-menu-should-call-onhostdestroy-or-any-other-event-to-detect-reload

ProductPains helps the community prioritize the most important issues thanks to its voting feature.
It is easy to use - just login with GitHub.

Also, if this issue is a bug, please consider sending a PR with a fix.
We're a small team and rely on the community for bug fixes of issues that don't affect fb apps.

[charpeni]

@facebook-github-bot close

[facebook-github-bot]

@charpeni tells me to close this issue. If you think it should still be opened let us know why.

[grabbou]

This is unfortunate. Some sort of event should be called to clean up resources.

[mauron85]

IMO unfortunate is also approach how reported issues are just closed without any comment. I appreciate any guy who is working hard on react-native, but for me as reporter and plugin developer who is making plugins in his free time (unpaid) this approach is very frustrating.

[ofirdagan]

+1

[lucasbento]

@mauron85: how did you end up solving this?

[mauron85]

@lucasbento I don't think I actually solved satisfactorily. I just fixed the crash but lot problems remained.

[lucasbento]

@mauron85, @grabbou, @charpeni: can this issue be reopened? I think it's valid, useful and should be implemented at some point.

[mauron85]

@lucasbento. I don't see any reopen button here. So I can not.

[charpeni]

@lucasbento Sure, let's reopen it.

[garrettseward]

how is this still not fixed? it surely can't be difficult to provide SOME kind of event when the app has been reloaded from the dev menu

[CrazyPython]

I suppose it may be difficult to send an event to the JS side, because it would require the native side to wait synchronously for the JS call stack to complete, something React Native is not (yet) designed for. Perhaps this functionality will be available after React Native's is redesigned internally.

[mauron85]

I was not talking about js side, rather native java. Add event onBeforeDestroy (or similarly named) to LifecycleEventListener.java interface and of course call that method on the caller side (whatever that java class is).

[CrazyPython]

I'm aware.

[stale[bot]]

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

[stale[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.

[grabbou]

I am reopening this. It's still an issue and we need a way to clean up resources when we reload the app. It's been really hard for me to clean up resources. If there's anyone knowing a better workaround, please let me know.

CC: @kmagiera maybe you happen to have a better context here?

[ferrannp]

So basically I think the behavior is correct (onHostDestroy not called) because the Host (Activity) is not destroyed. For what we talked though, I'd be nice if we could nootify native modules if the JS has been reloaded. This is of course, a DEV only issue.

[dulmandakh]

IIRC, there is no default way to notify native module if JS reloaded. But you can have native method and call it on componentDidMount() a component to do the binding.