Hero: crash in Hero.shared.end(), when pan too quickly

[PatrickSCLin]

when I pan an imageView in my app too quickly, it causes app crash sometimes,

context: HeroContext! is null

fatal error: unexpectedly found nil while unwrapping an Optional value
2017-05-16 10:58:52.556929+0800 soocii[44033:7935298] fatal error: unexpectedly found nil while unwrapping an Optional value

[PatrickSCLin]

it is easy to reproduce in Hero samples.

just pan to dismiss an imageView in ImageViewController in super big velocity

then app crashes

[andreamazz]

I can confirm that the issue is here:
https://github.com/lkzhao/Hero/blob/master/Sources/Hero.swift#L210

there are cases in which the complete(finished:) function is called twice, once with finished set to true, and then to false. On complete the context (and other variables that are declared as IUOs) is set to nil:
https://github.com/lkzhao/Hero/blob/master/Sources/HeroBaseController.swift#L356

I'd suggest to drop the Implicitly unwrapped optionals all together.
I'd love to hear what you think @lkzhao
Let me know if you need any help with this.

[PatrickSCLin]

any news ?

[BilalReffas]

Running on the same issue

[lkzhao]

Can you guys try the release-1.0 branch? let me know if the problem still persists.

pod "Hero", :git => 'https://github.com/lkzhao/Hero.git', :branch => "release-1.0"

[BilalReffas]

@lkzhao I will check it and let you know :)

[BilalReffas]

I have some problems to update the framework using the brach.. I'm using Carthage.. :/
github "lkzhao/Hero" "release-1.0"

[andreamazz]

@lkzhao
I'm still experiencing this issue, there's a call to animate when context is already nil, being an IUO it crashes.

[BilalReffas]

@andreamazz do you use Carthage?

[andreamazz]

Nope, CocoaPods

[BilalReffas]

Looks like Hero.shared.end() is away right?
Replace with this Hero.shared.animationEnded(true) right?

[andreamazz]

Replace with this Hero.shared.animationEnded(true) right?

I use Hero.shared.finish()

[PatrickSCLin]

it still can reproduce in branch of release 1.0

[BilalReffas]

Thank you @andreamazz

[BilalReffas]

Same for me @PatrickSCLin @andreamazz 😕

[ahartwel]

If you check out this commit (https://github.com/swarmnyc/Hero/commit/a08a6e4f647e26cd5954e690c73da454e661108a) in my forked version of hero, wrapping end and cancel in DispatchQueue.main.async blocks solved the crash when dragging/tapping too quickly for me

[PatrickSCLin]

any updates ? Hero is awesome framework but I can't use in production product with this issue

[SwiftyFlow]

This seams to fix it... (refer to //HOW TO FIX THE CRASH)

func pan() {
    let translation = panGR.translation(in: nil)
    let progress = translation.y / 2 / collectionView!.bounds.height
    switch panGR.state {
    case .began:
        //HOW TO FIX THE CRASH
        collectionView?.removeGestureRecognizer(panGR)   

        //then dismiss view
        SuperHero(aView: self, id: ImageViewControllerFor, backColor: UIColor.black, transition: HeroDefaultAnimationType.fade)//.zoomSlide(direction: .right))

        //avoid switching more into panGR.state
        break
    case .changed:
        Hero.shared.update(progress: Double(progress))
        if let cell = collectionView?.visibleCells[0]  as? ScrollingImageCell {
            let currentPos = CGPoint(x: translation.x + view.center.x, y: translation.y + view.center.y)
            Hero.shared.apply(modifiers: [.position(currentPos)], to: cell.imageView)
        }
    default:
        if progress + panGR.velocity(in: nil).y / collectionView!.bounds.height > 0.3 {
            Hero.shared.end()
        } else {
            Hero.shared.cancel()
        }
    }
}

BTW I can't resist:

func SuperHero(aView: UIViewController, id: String, backColor: UIColor, transition: HeroDefaultAnimationType) {
    let vc = aView.storyboard!.instantiateViewController(withIdentifier: id)
    Hero.shared.disableDefaultAnimationForNextTransition()
    Hero.shared.setContainerColorForNextTransition(backColor)
    Hero.shared.setDefaultAnimationForNextTransition(transition)
    aView.hero_replaceViewController(with: vc)
}

[ajpallares]

I also found the same bug in Hero.
When you call Hero.shared.end(), then, in line 253 of Hero.swift, the toViewController is set to nil.
If the call Hero.shared.end() occurs very soon after popViewController() was called (which can be achieved by quickly panning and releasing right after), line 253 gets executed before the start() method of Hero executes line 178:

self.animate()

where, in line 187 calls

context.unhide(view: toView)

Taking into account that the toView property is computed by

internal var toView: UIView { return toViewController!.view }

This call fails because toViewController is being unwrapped while it is nil.

I could find a temporary solution by slightly delaying the call to Hero.shared.end().

DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 0.01, execute: {
    Hero.shared.end()
})

It looks like this way, Hero has enough time to finish the call of the start() method before end() is executed. However, this is not the desired solution.

[k-nar]

I found that this bug occurred more often when there were some background thread doing heavy works (dynamic blurring in my case)
following @ajpallares comment I do

func heroSafeFinish()
{
        if(Hero.shared.state != .animating)
        {
            //anim did no start, wait a moment before finish
            DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 0.01, execute: {
                self.heroSafeFinish()
            })
        }
        else
        {
            Hero.shared.finish();
        }
    }

[caesss]

I implemented the solution by @ajpallares and it worked like a charm. Thank you very much!!!