Skip to content

Commit

Permalink
Avoid infinite loop by adding a parameter on whether or not the statu…
Browse files Browse the repository at this point in the history
…s should be recomputed

The current code has an infinite loop if there is a problem with the location settings or permission.
In that case, we generate a `CFCTransitionGeofenceCreationError`

```
    else {
        /*
         Log.i(ctxt, TAG, "location settings are valid, but location permission is not, generating tracking error and visible notification");
         Log.i(ctxt, TAG, "curr status check results = " +
                " loc permission, motion permission, notification, unused apps "+ Arrays.toString(allOtherChecks));
         */
        // Should replace with TRACKING_ERROR but looks like we
        // don't have any
        [[NSNotificationCenter defaultCenter]
            postNotificationName:CFCTransitionNotificationName
                object:CFCTransitionGeofenceCreationError];
        [self generateOpenAppSettingsNotification];
```

which should
cause the app go to into the start state.

```
    } else if ([transition isEqualToString:CFCTransitionGeofenceCreationError]) {
        [self setState:kStartState];
```

However, with 2fc078b, we now check the app state in `setState`.

```
  -(void)setState:(TripDiaryStates) newState {
...
     [SensorControlBackgroundChecker checkAppState];
```

So we end up with an infinite loop:
- check app settings
- permission checks fail
- geofence creation error
- set state to start

- check app settings
- permission checks fail
- geofence creation error
- set state to start

- check app settings
...

On android, we handle this by including a parameter that indicates whether the
app status should be recomputed or not (in 9560859)

This commit makes similar changes to iOS. On android, we would avoid
recomputing only once and that would be the case for iOS as well.

```
             Log.i(this, TAG, "Already in the start state, so going to stay there");
-            setNewState(mCurrState);
+            setNewState(mCurrState, false);
             return;
```

Testing done:
- No infinite loop on start (when we have no permissions)
  • Loading branch information
shankari committed Feb 15, 2022
1 parent ca979bd commit c2c58be
Showing 1 changed file with 17 additions and 15 deletions.
32 changes: 17 additions & 15 deletions src/ios/Location/TripDiaryStateMachine.m
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ - (id) init {
it easier for us to ignore silent push as well as the transitions generated from here.
*/
[TripDiaryActions oneTimeInitTracking:CFCTransitionInitialize withLocationMgr:self.locMgr];
[self setState:kOngoingTripState];
[self setState:kOngoingTripState withChecks:TRUE];
[TripDiaryActions startTracking:CFCTransitionExitedGeofence withLocationMgr:self.locMgr];
}

Expand Down Expand Up @@ -211,7 +211,7 @@ -(void)handleTransition:(NSString*) transition withUserInfo:(NSDictionary*) user
}
}

-(void)setState:(TripDiaryStates) newState {
-(void)setState:(TripDiaryStates) newState withChecks:(BOOL) doChecks {
NSUserDefaults *defaults = [NSUserDefaults standardUserDefaults];
[defaults setInteger:newState forKey:kCurrState];

Expand All @@ -221,8 +221,10 @@ -(void)setState:(TripDiaryStates) newState {
[TripDiaryStateMachine getStateName:newState]]];

self.currState = newState;
if (doChecks) {
[SensorControlBackgroundChecker checkAppState];
}
}

/*
* BEGIN: State transition handlers
Expand Down Expand Up @@ -251,7 +253,7 @@ -(void) handleStart:(NSString*) transition withUserInfo:(NSDictionary*) userInfo
// but if tracking is stopped, we can skip that, and then if we turn it on again, we
// need to turn everything on here as well
[TripDiaryActions oneTimeInitTracking:transition withLocationMgr:self.locMgr];
[self setState:kOngoingTripState];
[self setState:kOngoingTripState withChecks:TRUE];
[TripDiaryActions startTracking:transition withLocationMgr:self.locMgr];
}
} else if ([transition isEqualToString:CFCTransitionRecievedSilentPush]) {
Expand All @@ -260,26 +262,26 @@ -(void) handleStart:(NSString*) transition withUserInfo:(NSDictionary*) userInfo
} else if ([transition isEqualToString:CFCTransitionInitComplete]) {
// Geofence has been successfully created and we are inside it so we are about to move to
// the WAITING_FOR_TRIP_START state.
[self setState:kWaitingForTripStartState];
[self setState:kWaitingForTripStartState withChecks:TRUE];
} else if ([transition isEqualToString:CFCTransitionGeofenceCreationError]) {
// if we get a geofence creation error, we stay in the start state.
NSLog(@"Got transition %@ in state %@, staying in %@ state",
transition,
[TripDiaryStateMachine getStateName:self.currState],
[TripDiaryStateMachine getStateName:self.currState]);
[SensorControlBackgroundChecker checkAppState];
[self setState:kStartState withChecks:FALSE];
} else if ([transition isEqualToString:CFCTransitionExitedGeofence]) {
[TripDiaryActions startTracking:transition withLocationMgr:self.locMgr];
[TripDiaryActions deleteGeofence:self.locMgr];
[[NSNotificationCenter defaultCenter] postNotificationName:CFCTransitionNotificationName
object:CFCTransitionTripStarted];
} else if ([transition isEqualToString:CFCTransitionTripStarted]) {
[self setState:kOngoingTripState];
[self setState:kOngoingTripState withChecks:TRUE];
} else if ([transition isEqualToString:CFCTransitionForceStopTracking]) {
// One would think that we don't need to deal with anything other than starting from the start
// state, but we can be stuck in the start state for a while if it turns out that the geofence is
// not created correctly. If the user forces us to stop tracking then, we still need to do it.
[self setState:kTrackingStoppedState];
[self setState:kTrackingStoppedState withChecks:TRUE];
} else if ([transition isEqualToString:CFCTransitionStartTracking]) {
// One would think that we don't need to deal with anything other than starting from the start
// state, but we can be stuck in the start state for a while if it turns out that the geofence is
Expand Down Expand Up @@ -317,7 +319,7 @@ - (void) handleWaitingForTripStart:(NSString*) transition withUserInfo:(NSDicti
object:CFCTransitionTripStarted];
}
} else if ([transition isEqualToString:CFCTransitionTripStarted]) {
[self setState:kOngoingTripState];
[self setState:kOngoingTripState withChecks:TRUE];
} else if ([transition isEqualToString:CFCTransitionRecievedSilentPush]) {
// Let's push any pending changes since we know that the trip has ended
[[TripDiaryActions pushTripToServer] continueWithBlock:^id(BFTask *task) {
Expand All @@ -335,7 +337,7 @@ - (void) handleWaitingForTripStart:(NSString*) transition withUserInfo:(NSDicti
} else if ([transition isEqualToString:CFCTransitionForceStopTracking]) {
[TripDiaryActions resetFSM:transition withLocationMgr:self.locMgr];
} else if ([transition isEqualToString:CFCTransitionTrackingStopped]) {
[self setState:kTrackingStoppedState];
[self setState:kTrackingStoppedState withChecks:TRUE];
} else {
NSLog(@"Got unexpected transition %@ in state %@, ignoring",
transition,
Expand Down Expand Up @@ -405,7 +407,7 @@ - (void) handleOngoingTrip:(NSString*) transition withUserInfo:(NSDictionary*) u
// so we can use visits for the trip start detection, so we are also
// about to move to the WAITING_FOR_TRIP_START state
// TODO: Should this be here, or in EndTripTracking
[self setState:kWaitingForTripStartState];
[self setState:kWaitingForTripStartState withChecks:TRUE];
[[BEMServerSyncCommunicationHelper backgroundSync] continueWithBlock:^id(BFTask *task) {
[[BuiltinUserCache database] checkAfterPull];
[LocalNotificationManager addNotification:[NSString stringWithFormat:
Expand All @@ -417,11 +419,11 @@ - (void) handleOngoingTrip:(NSString*) transition withUserInfo:(NSDictionary*) u
}];
} else if ([transition isEqualToString:CFCTransitionGeofenceCreationError]) {
// setState will call SensorControlBackgroundChecker checkAppState by default
[self setState:kStartState];
[self setState:kStartState withChecks:FALSE];
} else if ([transition isEqualToString:CFCTransitionForceStopTracking]) {
[TripDiaryActions resetFSM:transition withLocationMgr:self.locMgr];
} else if ([transition isEqualToString:CFCTransitionTrackingStopped]) {
[self setState:kTrackingStoppedState];
[self setState:kTrackingStoppedState withChecks:TRUE];
} else {
NSLog(@"Got unexpected transition %@ in state %@, ignoring",
transition,
Expand All @@ -441,14 +443,14 @@ - (void) handleTrackingStopped:(NSString*) transition withUserInfo:(NSDictionary
[LocalNotificationManager addNotification:[NSString stringWithFormat:
@"Found unexpected VISIT_STARTED transition while tracking was stopped, stop everything again"]
showUI:TRUE];
[self setState:kTrackingStoppedState];
[self setState:kTrackingStoppedState withChecks:TRUE];
[TripDiaryActions resetFSM:transition withLocationMgr:self.locMgr];
} else if ([transition isEqualToString:CFCTransitionForceStopTracking]) {
[TripDiaryActions resetFSM:transition withLocationMgr:self.locMgr];
} else if ([transition isEqualToString:CFCTransitionTrackingStopped]) {
[self setState:kTrackingStoppedState];
[self setState:kTrackingStoppedState withChecks:TRUE];
} else if ([transition isEqualToString:CFCTransitionStartTracking]) {
[self setState:kStartState];
[self setState:kStartState withChecks:TRUE];
// This will run the one time init tracking as well as try to create a geofence
// if we are moving, then we will be outside the geofence... the existing state machine will take over.
[[NSNotificationCenter defaultCenter] postNotificationName:CFCTransitionNotificationName
Expand Down

0 comments on commit c2c58be

Please sign in to comment.