Skip to content
This repository has been archived by the owner on Apr 23, 2023. It is now read-only.

Simplify service shutdown to prevent crash on screen rotation #180

Merged
merged 1 commit into from
Apr 7, 2017

Conversation

ecgreb
Copy link
Collaborator

@ecgreb ecgreb commented Apr 5, 2017

Overview

Simplifies service shutdown process to prevent race condition that leads to a crash when the device is rotated.

Proposed Changes

Removes FusedLocationProviderServiceImpl#shutdown(). This method was previously invoked when the system called FusedLocationProviderService#onDestroy() and did two things:

  1. Clear all location requests by calling LocationEngine#setRequest(null).
  2. Clear all location clients from the client manager by calling LostClientManager#shutdown().

The problem is that FusedLocationProviderService#onDestroy() would be invoked asynchronously by the system after rotating the device. By the time onDestroy() fires the client application may have already re-connected the location client and/or made a new request for location updates.

If onDestroy() fires sometime between connecting the client and requesting updates it would lead to a crash similar those reported in #170.

Fortunately it seems neither of the operations in FusedLocationProviderServiceImpl#shutdown() are needed since both are already completed by the time onDestroy() is invoked by the system:

  1. Anytime a location request is removed checkAllListenersPendingIntentsAndCallbacks() is called which calls LocationEngine#setRequest(null) if all listeners have been removed (a prerequisite to stopping the service).
  2. In LostApiClientImpl#disconnect() the fused location service is only shutdown if there are no remaining clients so clearing the clients again when the service is destroyed is unnecessary.

Other Changes and Cleanup

  • Removes public method shutdown() from ClientManager interface.
  • Replaces with package private method LostClientManager#clearClients() visible for testing only.
  • Adds @Override annotations to ClientManager interface methods implemented in LostClientManager.
  • Shortens request intervals in MultipleLocationListenerSingleClientActivity to speed up feedback loop.

Fixes #170

@ecgreb
Copy link
Collaborator Author

ecgreb commented Apr 5, 2017

@westnordost can you see if this changes fixes the crash you reported in #170 (and doesn't introduce any side effects) since your stack trace was somewhat different from what we are able to reproduce with the Lost demo app?

@ecgreb
Copy link
Collaborator Author

ecgreb commented Apr 5, 2017

I built StreetComplete locally using a Lost 2.2.1-SNAPSHOT created from this branch and it does indeed seem to solve the crash introduced in version 2.2.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants