venmo / VENTouchLock

A Touch ID and Passcode framework used in the Venmo app.
MIT License
964 stars 114 forks source link

iPhone freeze #24

Closed cpfrndz closed 9 years ago

cpfrndz commented 9 years ago

My iPhone is freezes sometime when I canceled alert of touch ID.

dasmer commented 9 years ago

Are you using 1.0.3?

meoz commented 9 years ago

I'm using 1.0.4 and I have the same problem. When cancelling the alert for touch ID, the phone freezes, even the home button seems to do nothing. Only way to get out of the app is to use the on/off button on the iphone.

This used to work fine, (I guess version 1.0.2 and older)

zinthemoney commented 9 years ago

+1

dasmer commented 9 years ago

I pushed v1.0.5 today. This should have fixed the issue.

Please let me know if you continue to see the issue when updating to v1.0.5 or if the issue seems resolved.

Thank you :octocat:

lewis-smith commented 9 years ago

First up thanks for an amazing set of code! I have updated from pods today and I have an issue which is either the same or related. My steps are:

If I lock the phone and then go back to the app it works fine (it's the only way out of this 'freeze') and if I launch the app from scratch it works fine.

I've not extended VENTouchLockSplashViewController and am using that for now, but will probably change in the future.

This doesn't happen in the sample app so it must be something in my code so I'll keep looking, any advice in the right direction thoroughly appreciated.

My appDelegate code:

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{

    [Crashlytics startWithAPIKey:@"hidden"];

    NSShadow *shadow = [NSShadow new];
    [shadow setShadowColor:[UIColor clearColor]];
    [shadow setShadowOffset:CGSizeMake(0.0f, -1.0f)];

    [[UIBarButtonItem appearance] setTitleTextAttributes:@{NSForegroundColorAttributeName: [UIColor whiteColor],
                                                           NSFontAttributeName: [UIFont fontWithName:@"Avenir-Book" size:17],
                                                           NSShadowAttributeName: shadow} forState:UIControlStateNormal];

    [[UINavigationBar appearance] setTitleTextAttributes:@{NSForegroundColorAttributeName: [UIColor whiteColor],
                                                           NSFontAttributeName: [UIFont fontWithName:@"Avenir-Medium" size:24],
                                                           NSShadowAttributeName: shadow}];

    [[UINavigationBar appearance] setBarTintColor:[UIColor colorWithRed:0.6 green:0 blue:0.39 alpha:1]];

    [[VENTouchLock sharedInstance] setKeychainService:@"com.me"
                                      keychainAccount:@"com.me"
                                        touchIDReason:@"Unlock App"
                                 passcodeAttemptLimit:3
                            splashViewControllerClass:[VENTouchLockSplashViewController class]];

    return YES;
}
zinthemoney commented 9 years ago

@dasmer, the freeze is likely caused by showUnlockAnimated: being call twice. In VENTouchLockSplashViewController, both viewDidLoad and appWillEnterForeground are calling it. Here are some error descriptions when evaluatePolicy:localizedReason:reply: is sent to a LAContext instance. Assuming VENTouchLock isn't the only library accessing the KeyChain when app resumes.

Error Domain=com.apple.LocalAuthentication Code=-4 "Canceled by another authentication." UserInfo=0x17046b7c0 {NSLocalizedDescription=Canceled by another authentication.}

Error Domain=com.apple.LocalAuthentication Code=-1000 "Pending UI mechanism already set." UserInfo=0x174272e00 {NSLocalizedDescription=Pending UI mechanism already set.}

You moved the showUnlockAnimated: around on commit 37134378cb1b6b3c601cffaf9da3a07f3e6676ed. What was your reason behind that?

lewis-smith commented 9 years ago

@zinthemoney I think you are on the right track. In

- (void)appWillEnterForeground
{
    if (!self.presentedViewController) {
        [self showUnlockAnimated:NO];
    }
}

self.presentedViewController is null when showing the touch id input screen as the touch id dialog is not displayed using presentViewController:animated:completion:

So I guess storing a flag in VENTouchLock something like: isShowingTouchId which can then be checked in appWillEnterForeground would prevent the crash. Any one got anything neater?

dasmer commented 9 years ago

@zinthemoney @lewis-smith I've been looking into this and it seems like the freeze is due to LAContext being invoked twice and / or invoking LAContext in the background. I haven't been able to consistently reproduce the freeze so it seems like it is probably some sort of race condition.

@zinthemoney The reason I moved showUnlockAnimated: in https://github.com/venmo/VENTouchLock/commit/37134378cb1b6b3c601cffaf9da3a07f3e6676ed is because when it was in the completion handler it was being called too early (When presenting a view controller while the app is going into background, the completionHandler of presentViewController:animated:completion: is for some reason called before the presented view controller's viewDidLoad is called.). I thought this might have fixed the freeze issue, though it seems to have abated the race condition.

When it does freeze I do get error:

Error Domain=com.apple.LocalAuthentication Code=-1000 "Pending UI mechanism already set." UserInfo=0x174272e00 {NSLocalizedDescription=Pending UI mechanism already set.}

@lewis-smith Though its not pretty, maybe storing a flag for isShowingTouchId is a good idea.

I found a related stackoverflow question to this issue: http://stackoverflow.com/questions/26463196/touch-id-causing-app-to-become-non-responsive

I'm going to attempt a solution in a beta branch. When I'm done it would be helpful if you guys tested in your apps as well.

Thanks so much for your help and input so far :octocat:

dasmer commented 9 years ago

@zinthemoney @lewis-smith please take a look at #31

I manually tested it in the Sample App and Venmo app, and it seems to be robust (observed 0 freezes after unlocking via Touch ID multiple times, trying a variety of different steps to unlock)

zinthemoney commented 9 years ago

the fix is good. cheers!

lewis-smith commented 9 years ago

Yeah good for me too, great work, thanks :)

dasmer commented 9 years ago

Great. Thank you guys!