ra1028/Carbon

cellNode(at: indexPath) Fatal error: Index out of range

Fabezi opened this issue · 1 comments

Fabezi commented

Checklist

Expected Behavior

Should not crash

Current Behavior

Rapid updates between these two type of reload methods cause datasource to be inconsistent
https://github.com/ra1028/Carbon/blob/master/Sources/Updaters/UICollectionViewUpdater.swift#L59-L64
https://github.com/ra1028/Carbon/blob/master/Sources/Updaters/UICollectionViewUpdater.swift#L70-L75

and will sometimes, very rarely, cause cellNode(at: indexPath) below to throw an fatal arror
https://github.com/ra1028/Carbon/blob/master/Sources/Updaters/UICollectionViewUpdater.swift#L168

Steps to Reproduce

Attached simple example project. You may have to run it a few times.
CarbonCrash.zip

Screenshot 2020-10-29 at 21 45 39

Environments

  • Carbon version: Master
  • Swift version: 5.3
  • iOS version: 14.1
  • Xcode version: 12
  • Devices/Simulators: All
  • CocoaPods/Carthage version: SPM
xinzhengzhang commented

I've fixed this problem myself for a long time, but it seems not to be maintained at the moment, I'll post the patch.

diff --git a/Sources/Updaters/UICollectionViewUpdater.swift b/Sources/Updaters/UICollectionViewUpdater.swift
index bbc4db5..e3f8352 100644
--- a/Sources/Updaters/UICollectionViewUpdater.swift
+++ b/Sources/Updaters/UICollectionViewUpdater.swift
@@ -155,6 +155,7 @@ open class UICollectionViewUpdater<Adapter: UICollectionViewAdapter>: Updater {
             target.performBatchUpdates({
                 for kind in adapter.registeredSupplementaryViewKinds(for: target) {
                     for indexPath in target.indexPathsForVisibleSupplementaryElements(ofKind: kind) {
+                        guard indexPath.section != NSNotFound, indexPath.row != NSNotFound else { continue }
                         guard let node = adapter.supplementaryViewNode(forElementKind: kind, collectionView: target, at: indexPath) else {
                             continue
                         }
@@ -165,6 +166,8 @@ open class UICollectionViewUpdater<Adapter: UICollectionViewAdapter>: Updater {
                 }
 
                 for indexPath in target.indexPathsForVisibleItems {
+                    guard indexPath.section != NSNotFound, indexPath.row != NSNotFound else { continue }
+
                     let cellNode = adapter.cellNode(at: indexPath)
                     let cell = target.cellForItem(at: indexPath) as? ComponentRenderable
                     cell?.render(component: cellNode.component)
diff --git a/Sources/Updaters/UITableViewUpdater.swift b/Sources/Updaters/UITableViewUpdater.swift
index 1b914f6..a7ef2fa 100644
--- a/Sources/Updaters/UITableViewUpdater.swift
+++ b/Sources/Updaters/UITableViewUpdater.swift
@@ -203,6 +203,8 @@ open class UITableViewUpdater<Adapter: UITableViewAdapter>: Updater {
                 }
 
                 for indexPath in target.indexPathsForVisibleRows ?? [] {
+                    guard indexPath.section != NSNotFound, indexPath.row != NSNotFound else { continue }
+
                     let cellNode = adapter.cellNode(at: indexPath)
                     let cell = target.cellForRow(at: indexPath) as? ComponentRenderable
                     cell?.render(component: cellNode.component)

By the way, some naming changes due to swift changes, but just renamed.

diff --git a/Sources/Adapters/Adapter.swift b/Sources/Adapters/Adapter.swift
index be18941..6c91137 100644
--- a/Sources/Adapters/Adapter.swift
+++ b/Sources/Adapters/Adapter.swift
@@ -1,7 +1,7 @@
 import Foundation
 
 /// Represents an adapter that holds data to be rendered.
-public protocol Adapter: class {
+public protocol Adapter: AnyObject {
     /// The data to be rendered.
     var data: [Section] { get set }
 }
diff --git a/Sources/FunctionBuilder/CellsBuilder.swift b/Sources/FunctionBuilder/CellsBuilder.swift
index 2638d8b..a75c88d 100644
--- a/Sources/FunctionBuilder/CellsBuilder.swift
+++ b/Sources/FunctionBuilder/CellsBuilder.swift
@@ -2,7 +2,7 @@
 // swiftlint:disable function_parameter_count
 
 /// The custom parameter attribute that constructs cells from multi-statement closures.
-@_functionBuilder
+@resultBuilder
 public struct CellsBuilder: CellsBuildable {
     @usableFromInline
     internal var cellNodes: [CellNode]
diff --git a/Sources/FunctionBuilder/SectionsBuilder.swift b/Sources/FunctionBuilder/SectionsBuilder.swift
index e74b37f..1369e4d 100644
--- a/Sources/FunctionBuilder/SectionsBuilder.swift
+++ b/Sources/FunctionBuilder/SectionsBuilder.swift
@@ -2,7 +2,7 @@
 // swiftlint:disable function_parameter_count
 
 /// The custom parameter attribute that constructs sections from multi-statement closures.
-@_functionBuilder
+@resultBuilder
 public struct SectionsBuilder: SectionsBuildable {
     @usableFromInline
     internal var sections: [Section]
diff --git a/Sources/Interfaces/ComponentRenderable.swift b/Sources/Interfaces/ComponentRenderable.swift
index 93e4d47..2613018 100644
--- a/Sources/Interfaces/ComponentRenderable.swift
+++ b/Sources/Interfaces/ComponentRenderable.swift
@@ -1,7 +1,7 @@
 import UIKit
 
 /// Represents a container that can render a component.
-public protocol ComponentRenderable: class {
+public protocol ComponentRenderable: AnyObject {
     /// The container view to be render a component.
     var componentContainerView: UIView { get }
 }
diff --git a/Tests/TestTools.swift b/Tests/TestTools.swift
index 437fadf..9c8d5d2 100644
--- a/Tests/TestTools.swift
+++ b/Tests/TestTools.swift
@@ -489,7 +489,7 @@ struct Pair<T, U> {
 extension Pair: Equatable where T: Equatable, U: Equatable {}
 
 /// Protocol for `UIView` utility.
-protocol UIViewConvertible: class {
+protocol UIViewConvertible: AnyObject {
     var uiView: UIView { get }
 }