1221247 - Crash in accessing some Sync folders

Status: RESOLVED FIXED

(Firefox for iOS :: Home screen, defect)

Description

[Aaron Train [:aaronmt]]

Built (27d666). Launched. Set up Sync, backed out of Settings. Tapped Bookmarks -> Desktop Bookmarks -. Mobile Bookmarks


(lldb) bt
* thread #1: tid = 0x2402e7, 0x00000001016244fc libswiftCore.dylib`function signature specialization <Arg[0] = Exploded, Arg[1] = Exploded, Arg[2] = Dead, Arg[3] = Dead> of Swift._fatalErrorMessage (Swift.StaticString, Swift.StaticString, Swift.StaticString, Swift.UInt) -> () + 44, queue = 'com.apple.main-thread', stop reason = EXC_BREAKPOINT (code=1, subcode=0x1016244fc)
  * frame #0: 0x00000001016244fc libswiftCore.dylib`function signature specialization <Arg[0] = Exploded, Arg[1] = Exploded, Arg[2] = Dead, Arg[3] = Dead> of Swift._fatalErrorMessage (Swift.StaticString, Swift.StaticString, Swift.StaticString, Swift.UInt) -> () + 44
    frame #1: 0x0000000100b816c0 Storage`Storage.SQLiteBookmarkFolder.subscript.getter : (index=1, self=0x0000000128058de0) -> Storage.BookmarkNode + 368 at SQLiteBookmarks.swift:41
    frame #2: 0x000000010033c3b4 Client`Client.BookmarksPanel.tableView (tableView=0x0000000127159000, indexPath=0xc000000000200016, self=0x0000000127e738b0)(__ObjC.UITableView, cellForRowAtIndexPath : __ObjC.NSIndexPath) -> __ObjC.UITableViewCell + 168 at BookmarksPanel.swift:113
    frame #3: 0x000000010033ce7c Client`@objc Client.BookmarksPanel.tableView (Client.BookmarksPanel)(__ObjC.UITableView, cellForRowAtIndexPath : __ObjC.NSIndexPath) -> __ObjC.UITableViewCell + 92 at BookmarksPanel.swift:0
    frame #4: 0x00000001884f220c UIKit`-[UITableView _createPreparedCellForGlobalRow:withIndexPath:willDisplay:] + 688
    frame #5: 0x00000001884f2364 UIKit`-[UITableView _createPreparedCellForGlobalRow:willDisplay:] + 80
    frame #6: 0x00000001884e17b8 UIKit`-[UITableView _updateVisibleCellsNow:isRecursive:] + 2440
    frame #7: 0x00000001884f6f0c UIKit`-[UITableView _performWithCachedTraitCollection:] + 104
    frame #8: 0x000000018828f22c UIKit`-[UITableView layoutSubviews] + 176
    frame #9: 0x000000018819f7ac UIKit`-[UIView(CALayerDelegate) layoutSublayersOfLayer:] + 644
    frame #10: 0x000000018799eb58 QuartzCore`-[CALayer layoutSublayers] + 148
    frame #11: 0x0000000187999764 QuartzCore`CA::Layer::layout_if_needed(CA::Transaction*) + 292
    frame #12: 0x0000000187999624 QuartzCore`CA::Layer::layout_and_display_if_needed(CA::Transaction*) + 32
    frame #13: 0x0000000187998cc0 QuartzCore`CA::Context::commit_transaction(CA::Transaction*) + 252
    frame #14: 0x0000000187998a08 QuartzCore`CA::Transaction::commit() + 512
    frame #15: 0x00000001881959d8 UIKit`_afterCACommitHandler + 180
    frame #16: 0x0000000182bc7bd0 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 32
    frame #17: 0x0000000182bc5974 CoreFoundation`__CFRunLoopDoObservers + 372
    frame #18: 0x0000000182bc5da4 CoreFoundation`__CFRunLoopRun + 928
    frame #19: 0x0000000182af4ca0 CoreFoundation`CFRunLoopRunSpecific + 384
    frame #20: 0x000000018dd30088 GraphicsServices`GSEventRunModal + 180
    frame #21: 0x000000018820cffc UIKit`UIApplicationMain + 204
    frame #22: 0x00000001002c8ea0 Client`main + 528 at main.swift:22
    frame #23: 0x00000001980128b8 libdyld.dylib`start + 4

Comment 1

[Stephan Leroux [:sleroux]]

Looks like the fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1218826 didn't work. Aaron has a somewhat reliable STR so I'll dig into this.

Comment 2

[Aaron Train [:aaronmt]]

I hit this demonstrating to Stephan during a Sync and after a Sync. I attempted a different folder and hit it as well. Trying a third time after a Sync and I couldn't reproduce it again. During a Sync, I can hit this 100%.

Comment 3

[Stephan Leroux [:sleroux]]

Looks like this bug is being caused by an explicit unwrapping of nil here:

https://github.com/mozilla/firefox-ios/blob/master/Storage/SQL/SQLiteBookmarks.swift#L41

What's happening is that when we click on the cell, it's running cellForRowAtPath in the BookmarksPanel and providing it an out of bounds indexPath. In the case that I was able to reproduce, my 'News' bookmark folder has only one measly article but it tried to grab the 2nd entry which produced nil. 

What's strange is that cellForRowAtIndexPath would only get called for an out of bounds index if the numberOfRowsInSection was returning the incorrect number. If that's true, then it seems likely that there's a race going on between when we fetch the next/selected bookmark folder and when we reload the table view to update the number of rows in the section

https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Home/BookmarksPanel.swift#L67

This would also explain why it's rare to reproduce.

Comment 4

[Stephan Leroux [:sleroux]]

Attachment: Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/1218

Context:

When a UITableView attachs to the window for the first time, reloadData is called. If we haven't fetched our folder yet, then we will be using the previous folder's source instead which causes the data source to be out of sync with what we want to show. This patch simply adds a check to make sure we only return the source.current.count after we've fetched.

Comment 5

[Richard Newman [:rnewman]]

Attachment: Alternative patch.

Comment 6

[Stephan Leroux [:sleroux]]

Comment on attachment 8682740 [details] [review]
Alternative patch.

I like the alternative patch better than my boolean flag option. Lets go with that.

Comment 7

[Stephan Leroux [:sleroux]]

Merging this in for the build today.

79cae9af60146894bb6269e2a3e4ce00d150b939

Comment 8

[Richard Newman [:rnewman]]

Leaving sleroux as assigned so he gets blamed for the fallout ;)