52inc / Pulley

A library to imitate the iOS 10 Maps UI.
https://cocoapods.org/pods/Pulley
MIT License
2.02k stars 265 forks source link

Pulley hangs on iOS 14 Beta 4 #390

Open florianbuerger opened 4 years ago

florianbuerger commented 4 years ago

After upgrading to iOS 14 Beta 4, even the example projects won't launch anymore and the CPU is at 100%. I didn't have time to investigate further so I don't know if this is a bug from iOS 14 or Pulley. It worked fine until iOS 14 Beta 3.

Anyway, a short term fix I found is to slightly delay the layout code for Pulley with a good old DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) — which, in my initial testing — seems to resolve the issue. See for an example https://github.com/florianbuerger/Pulley/commit/ea6c287f4153e510ede4baffd0e3c96bb6a000e0

Just wanted to leave that here in case anyone else is running into it.

rafaelfrancisco-dev commented 4 years ago

This bug started showing up in 14b4, it's an auto layout loop caused by calling view.setNeedsLayout() inside of override open func viewDidLayoutSubviews() when setting the value on the variable currentDisplayMode. For now I've removed the view.setNeedsLayout() from the variable setter, but it might cause unexpected behavior when changing the drawer display mode after it is drawn.

My guess is that this bug is being caused by a faster UI drawing brought by the latest beta, which causes a new re-draw while the call to viewDidLayoutSubviews() hasn't finished yet, you can run your projects with the launch argument -UIViewLayoutFeedbackLoopDebuggingThreshold 100 to better debug this issue.

andreyz commented 4 years ago

Here's a good summary on how to debug auto-layout feedback loops.

pmacro commented 4 years ago

FYI still occurs on Beta 5.

pmacro commented 4 years ago

Around line 597:

changing this:

    public fileprivate(set) var currentDisplayMode: PulleyDisplayMode = .automatic {
        didSet {
            if self.isViewLoaded
            {
                self.view.setNeedsLayout()
            }

            if oldValue != currentDisplayMode
            {

to this:

    public fileprivate(set) var currentDisplayMode: PulleyDisplayMode = .automatic {
        didSet {
            if oldValue != currentDisplayMode
            {
                if self.isViewLoaded
                {
                    self.view.setNeedsLayout()
                }

Fixes it. I don't know this feature or codebase well enough to judge the implications of not running setNeedsLayout when old and new values are the same.

funkenstrahlen commented 4 years ago

I can confirm this issue. Still present on beta 5.

@pmacro Thank you for providing a proposal for a fix! Are you able to open a PR with the fix?

pmacro commented 4 years ago

@funkenstrahlen we only use drawer mode, so my workaround may not work in all cases. I think someone who knows the codebase, and all edge cases, or who has time to discover them, would be better placed to fix this. I posted the workaround mainly to help anyone else who's trying to test the rest of their app for iOS 14 launch.

ulmentflam commented 4 years ago

Hey! Thanks everyone for looking into this issue as I have not had a chance to dive into Xcode 12 yet. I have created the xcode12 branch to start looking into the beta issues with iOS 14 and have included the above patch for this issue, as well as the fix for if the drawer in panel mode and the display mode does not transition. Please continue to test on this branch and let me know what you run into in the betas leading up to the iOS 14 release.

funkenstrahlen commented 4 years ago

Thank you for providing the xcode12 branch with the fix. It does actually improve the situation as the app does not hang on startup anymore.

However when I try to manually change the pulley position state it still starts to hang in an infinite loop.

stevi-clue commented 4 years ago

Hi! When can we expect an updated version of the SDK with the fix?

ulmentflam commented 4 years ago

@florianbuerger is this infinite loop happening when you call setDrawerPosition(position, animated, completion?)?

florianbuerger commented 4 years ago

@ulmentflam I am not 100% sure what you mean. Do you mean in the patched version I linked in my first message?

We have been using Pulley with that fix for a while now and we didn't encounter any more problems. There are no hangs anymore. We are calling setDrawerPosition(position, animated, completion?) often and also directly after launching the app to restore a saved drawer position. We are also using the panel mode on iPad, no issues there as well.

I haven't had time to check out the xcode12 branch.

ulmentflam commented 4 years ago

I'm sorry @florianbuerger, I did mean to tag @funkenstrahlen in my question above who is testing on the xcode12 branch. However, it is good to know that you are not experiencing any other hanging issues with a slight delay in the layout code as I continue to investigate.

florianbuerger commented 4 years ago

@ulmentflam Ah, got it 😄 I'll see if I can test the xcode12 branch this week. Not sure if we need anything else from my fork of Pulley, it has been a while since I looked at that part of the app.

sohail-niazi commented 3 years ago

Hi, I have tried this branch in xcode 12 but app hangs on launch. but it is working fine with xcode 11.

Hey! Thanks everyone for looking into this issue as I have not had a chance to dive into Xcode 12 yet. I have created the xcode12 branch to start looking into the beta issues with iOS 14 and have included the above patch for this issue, as well as the fix for if the drawer in panel mode and the display mode does not transition. Please continue to test on this branch and let me know what you run into in the betas leading up to the iOS 14 release.

ulmentflam commented 3 years ago

@sohail-niazi are you using Xcode 12 Beta 6?

sohail-niazi commented 3 years ago

@sohail-niazi are you using Xcode 12 Beta 6?

yes it is Xcode 12 beta 6.

pmacro commented 3 years ago

Hi @ulmentflam, I fully understand all the moving parts here, and that this may be an unreasonable question! But given the announcement that iOS 14 will be released tomorrow, could you please share your thoughts on the timing of this fix? Do you plan to have a compatible release ASAP, or do you plan to hold off until there is more clarity on reported issues, such as @sohail-niazi's? That information would help me, and hopefully others, who plan to release iOS 14 updates for their apps ASAP. Thank you!

ulmentflam commented 3 years ago

@pmacro I am planning to hold off until there is more clarity on the reported issues before getting this branch out specifically. I would like to get more clarity on @sohail-niazi's and @funkenstrahlen's issues before I release (as on the sample project I have not been able to replicate either of their issues on this branch). I want to get a fix for this issue out as soon as possible, as soon as I have clarity on these existing issues. That leads me to my next question for @sohail-niazi, Can you replicate this issue in the demo project for me?

funkenstrahlen commented 3 years ago

Unfortunately I will not be able to provide further input to this issue. I decided to go with a completely different approach and use system default modal views instead of Pulley in my app as I am trying to get the app compatible with macOS. This will be more easy with default system components in my case.

pschneider commented 3 years ago

@ulmentflam We can reproduce this issue in two of our apps with latest stable version. After applying the xcode12 branch of this repo our views work as expected and we could so far not detect any other side effects.

sohail-niazi commented 3 years ago

@ulmentflam I have tried demo project for xcode12 branch. It works fine but still I cant make it work(my project) on ios14, ios13 is running fine. I have vereified xcode12 branch changes after pod install. But App just gets stuck on launch and no logs even.

UPDATE: Demo project too is hanging on Start ... Steps to reproduce error: 1 - UnComment programatical loading of pulleyViewController and add primary/drawer views 2 - set initial position .closed and app hangs.

ulmentflam commented 3 years ago

@sohail-niazi have you made any customizations or tweeks to Pulley, or have you subclassed it?

sohail-niazi commented 3 years ago

@sohail-niazi have you made any customizations or tweeks to Pulley, or have you subclassed it?

No.. I am using base PulleyViewController in my application.

I was able to reproduce error on demo project. Steps to reproduce error: 1 - UnComment programatical loading of pulleyViewController and add primary/drawer views 2 - set initial position .closed and app hangs.

ulmentflam commented 3 years ago

@sohail-niazi That's great! It looks like there is still an auto-layout feedback loop when the initial drawer position is set. I am looking into it now, thanks for the feedback!

sohail-niazi commented 3 years ago

@ulmentflam Apparently PulleyViewController is running in infinite loop. It's viewDidLayoutSubviews() method is getting called in loop... And getting this message in UIMainApplication after applying breakpoint in PulleyViewController : "Application violated contract by causing UIApplicationMain() to return. This incident will be reported."

sohail-niazi commented 3 years ago

@sohail-niazi That's great! It looks like there is still an auto-layout feedback loop when the initial drawer position is set. I am looking into it now, thanks for the feedback!

Much appreciated speedy response. Thanks...

ulmentflam commented 3 years ago

@funkenstrahlen I know you are no longer using Pulley for your application, but were you by chance changing the position of the drawer to or from the .closed position?

funkenstrahlen commented 3 years ago

@funkenstrahlen I know you are no longer using Pulley for your application, but were you by chance changing the position of the drawer to or from the .closed position?

Yes that's exactly what I did and that also caused the same 'hang' symptom.

ulmentflam commented 3 years ago

@funkenstrahlen Thank you! This auto-layout loop is related to when the view is laid out in the .closed position. For everyone on this branch or waiting on this issue, I have been debugging this loop and will get the xcode12 branch released as soon as there is a fix.

ulmentflam commented 3 years ago

I just pushed a theoretical fix to the xcode12 branch. If anyone continues to run into issues let me know ASAP, otherwise I will get a release out as soon as there is confirmation that this update is working.

RamblinWreck77 commented 3 years ago

Just found this and can confirm the tight loop in setDrawerPosition for iOS14 users. Testing the xcode 12 branch in a minute.

RamblinWreck77 commented 3 years ago

Well right off the bat, the xcode 12 branch does not tight loop and seems to be working :) So far so good!

ulmentflam commented 3 years ago

We have a official fix for the iOS 14 and Xcode 12 issues here. We can address any new issue related to the above fixes on the xcode12 branch. However to get something stable out sooner rather then later for iOS 14, the fixes in the xcode12 branch have been released. Pulley 2.8.2

sohail-niazi commented 3 years ago

@ulmentflam Thanks for new release. But The issue persists. Auto-layout is looping, it is looping for collapsed position so check of .closed is not working. drawerMaskingPath() is getting called from 3 different locations : 1 - maskBackgroundDimmingView() 2 - maskDrawerVisualEffectView() 3 - viewDidLayoutSubviews() on line 862

emresa37 commented 3 years ago

It's still has the issue. drawerMaskingPath() is looping somehow while setDrawerContentViewController()

ulmentflam commented 3 years ago

Gotcha, looking into it now

ulmentflam commented 3 years ago

@sohail-niazi and @emresa37 I just updated the xcode12 branch again with a potential fix. Could you both see if Auto-layout is still looping on drawerMaskingPath()?

sohail-niazi commented 3 years ago

@ulmentflam looking into it.

sohail-niazi commented 3 years ago

@ulmentflam still going in loop. Drawer's ViewDidLoad gets called already, so this check doesn't work. Same loop is going on..

RamblinWreck77 commented 3 years ago

2.8.2 works for me, no tight loop.

ulmentflam commented 3 years ago

@sohail-niazi Interesting, did you by chance replicate this on the sample app?

sohail-niazi commented 3 years ago

@ulmentflam it is not replicating on sample app. On my app, I have commented almost all extra code to just load pulley, but is stuck on launching screen

ulmentflam commented 3 years ago

@sohail-niazi Can you post a code snippet with how you are initializing Pulley, and also the output of running your app with https://www.hackingwithswift.com/articles/59/debugging-auto-layout-feedback-loops as I am not able to replicate on the sample app.

sohail-niazi commented 3 years ago

Issue is resolved in App. It is working fine now. The culprit was this code snippet:

 func drawerChangedDistanceFromBottom(drawer: PulleyViewController, distance: CGFloat, bottomSafeArea: CGFloat)
    {

        if distance > 0.0 {
            drawer.drawerBackgroundVisualEffectView = UIVisualEffectView(effect: UIBlurEffect(style: .dark))
        } else {
            drawer.drawerBackgroundVisualEffectView = UIVisualEffectView(effect: .none)
        }

    }

removing It worked as expected.

@ulmentflam Thank you very much for great support and response.

ulmentflam commented 3 years ago

@sohail-niazi No problem! Are you still on the xcode12 branch?

sohail-niazi commented 3 years ago

@ulmentflam Yes I am.

ulmentflam commented 3 years ago

@emresa37 Are you still experiencing your issue on the xcode12 branch?

emresa37 commented 3 years ago

@ulmentflam No it's fixed. Thank you for great support 👍

ovdm commented 3 years ago

@ulmentflam also experienced the issue. 100% CPU, loop on layoutSubview.. Just upgraded to master and seems to be working. Thanks for the support. Would have been virtually impossible for me to fix

pmacro commented 3 years ago

@ulmentflam I just wanted to confirm: is the xcode12 branch that everyone seems to be reporting as working, equal to the 2.8.3 release? Or is there something different in there that you're planning for a subsequent release?

And as an aside, thanks for all your work on this release, especially given the short time scale Apple gave us all to have solid iOS 14 versions of our libraries. 👏