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

iOS 12 MultipleSelectorRow Crash: Index out of bounds #1575

Closed
hstdt opened this issue Jun 13, 2018 · 8 comments
Closed

iOS 12 MultipleSelectorRow Crash: Index out of bounds #1575

hstdt opened this issue Jun 13, 2018 · 8 comments

Comments

@hstdt
Copy link

hstdt commented Jun 13, 2018

Xcode 10, iOS 12, MultipleSelectorRow will crash when I delete quickly. (iOS 11 simulator will not)

quick-crash:

eureka-ios12

slow:

eureka-ios12-slow

it could be a bug of system, report here for less confusing.

@mats-claassen
Copy link
Member

I can confirm I am seeing this issue. In the example you show I can see that tableView(_ tableView: UITableView, commit editingStyle: UITableViewCellEditingStyle, forRowAt indexPath: IndexPath) is called with an indexPath of (0, 4) while there are only 4 rows in the section.

This looks like an iOS bug. I think you should file a radar if there isn't one already.

As this is an edge case and we are still in beta for iOS 12 we will not work on a workaround for now.

@kanunnikau
Copy link

Hello. I am seeing a potentially related issue in earlier iOS versions (managed to reproduce it in iOS 11, but there doesn't seem to be anything special about it, could be reproducible in earlier versions too). How the crash happens for us:

  • set up default table with 4 cells in viewDidLoad. This results in Form => tableView.reloadData() called
    -- have one of the cells (at index 2 in our case, next to last) be optionally hidden, depending on certain conditions. It is visible by default
  • start loading data
  • receive server response, set conditional cell as hidden => crash

What I think is going on here is a classic race condition - UITableView is still being reloaded (and happily calling IndexPaths up to 4, as it was originally set up), but due to the hidden property observer the code in Section.hide(row:) removes one row during that process. Indices no longer match.
Our quick fix was to wrap the code changing the hidden condition with DispatchQueue.main.async, making sure that the cell is hidden only during next run loop, once table view finishes reloading. This kind of fix has to be applied to all cases when we can possibly hide/reveal rows. I'd prefer to have the library do it automatically. Would it be a reasonable fix to wrap all cases where kvoWrapper.rows are mutated (hidden/revealed/removed/appended) in DispatchQueue.main.async? I could prepare a merge request, but I'm not too familiar with the project.

@mats-claassen
Copy link
Member

What is weird about the first case is that the cell to be removed has index 2 but the function is called with index 4 while only one row had been removed.

Apart from that it seems that iOS definitely takes some time to actually remove a cell and if something happens during that time the indexPaths involved might go out of bounds.

I am not sure it is a good idea to change kvoWrappers.rows (or sections in the form) asynchronously because you might fall into a case where you append a section to a form and then call something on form.last but that section has not been added yet. Like:

form +++ Section()
form.last! <<< TextRow() // section might not have been added yet

@kanunnikau
Copy link

kanunnikau commented Jun 20, 2018

Apart from that it seems that iOS definitely takes some time to actually remove a cell

In my case, it was reloadData(), which is known to take a long time. Given that, I'm not too surprised to hear that removing cells and redrawing also takes an undefined amount of time. That's why begin/endUpdates (or a newer performBatchUpdates) exist. As your code is based on UITableView with such behaviour it is reasonable to assume that such flow would be problematic:

  • create/append content
  • redraw
  • change it immediately after

It appears that we have to resort to hacks with scheduling on the next runloop or use updating methods with completion blocks. What would be easier to implement given the current architecture?

but the function is called with index 4 while only one row had been removed.

Probably it is because the table is just getting around to drawing the last "Add New Tag" row from the original set of cells. This looks very similar to my case - have several cells, start drawing it, quickly remove (hide) one - crash while trying to finish drawing cells which should have been at the end of the original list

@mats-claassen
Copy link
Member

The crash with the MultipleSelectorRow does not happen in the drawing call but in tableView(_ tableView: UITableView, commit editingStyle: UITableViewCellEditingStyle, forRowAt indexPath: IndexPath) which is where it gets removed.
So we have two slightly different problems here.

@sc0rch
Copy link

sc0rch commented Sep 1, 2018

It happens in iOS 11.4 too (Simulator).

@hstdt
Copy link
Author

hstdt commented Apr 30, 2019

It seems to be fixed now. Closed.

@hstdt hstdt closed this as completed Apr 30, 2019
@racazoi
Copy link

racazoi commented Nov 1, 2019

iOS 13 has the same issue not always but it happens

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

No branches or pull requests

5 participants