Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crash after dismissing a presented UICollectionViewController where InfiniteCarousel is the controller's collection view #16

Open
klundberg opened this issue Dec 19, 2015 · 7 comments
Labels

Comments

@klundberg
Copy link

If you create a collection view controller where the InfiniteCarousel is set to be the collection view of said controller, and you then present/scroll/dismiss that view controller, the next time you try to focus something new, the app will crash at this line:

https://github.com/willowtreeapps/ouroboros/blob/develop/Ouroboros/InfiniteCarousel.swift#L92

The problem is that the focus engine seems to keep a reference to the Infinite carousel, and when you refocus, that reference is removed. UIScrollView's dealloc method manually sets datasource and delegate to nil, and that set here causes the overridden property for delegate to trigger the set, which tries to re-set super.delegate. Doing this causes a trap, since the view is being deallocated it is not allowed to be set to a weak reference which it is doing by setting super.delegate to self.

@ianterrell
Copy link
Contributor

Thank you for the report. Definitely sounds like a bug.

If you have a small sample project that demonstrates this issue it could definitely help speed up a fix. PRs also welcome of course. :)

@ianterrell ianterrell added the bug label Dec 21, 2015
@snofla
Copy link

snofla commented Jan 3, 2016

I fixed it by making deinit set a variable, and check in the offending lines for this variable. If variable is set I call the super implementation of the setter. HTH.

@klundberg
Copy link
Author

I haven't needed to use this recently, it was for a POC, but if I have time this weekend I'll try to make a sample proj for it and see if I can reproduce it there.

@ehynds
Copy link

ehynds commented Jan 27, 2016

@snofla mind sharing code for how you fixed this? I'm running into the issue as well.

@ianterrell
Copy link
Contributor

It sounds to me like @snofla was doing something like this:

ViewController {
  deinit {
    infiniteCarousel.deiniting = true
  }
}

InfiniteCarousel {
  var deiniting = false
  set {
    guard !deiniting else {
      super.delegate = newValue
      return
    }

    rootDelegate = newValue
    super.delegate = self
  }
}

In my mind the nil case might be the most relevant here... I'm not sure. If so, something like this could work:

set {
  guard let delegate = newValue else {
    rootDelegate = nil
    super.delegate = nil
    return`
  }

  rootDelegate = delegate
  super.delegate = self
}

We haven't run into this case yet and I've been too busy to repro it manually; if you attach a repro and a PR I will review and try to get it merged into a new release.

@snofla
Copy link

snofla commented Jan 27, 2016

@ianterrell Yes, the former, I'll try to come up with a reproducible scenario.

@BProg
Copy link

BProg commented Aug 25, 2016

I have resolved this bug on my fork. But a strange thing is that if I use InfiniteCarousel as framework, it still crashes, but when adding file directly to the project, it does not crash with the fix.

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

No branches or pull requests

5 participants