Wordpress-ios: Fatal Exception: NSRangeException in WPRichContentView deinit

[jkmassel]

From Fabric:

Fatal Exception: NSRangeException
*** NSRunStorage, _NSBlockNumberForIndex(): index (9223372036854722427) beyond array bounds (9223372036854722427)

Fatal Exception: NSRangeException
0  CoreFoundation                 0x22ded8ec4 __exceptionPreprocess
1  libobjc.A.dylib                0x22d0a9a50 objc_exception_throw
2  CoreFoundation                 0x22dddf594 -[NSCache init]
3  UIFoundation                   0x23855477c _NSBlockNumberForIndex
4  UIFoundation                   0x23854bdd4 _NSDeleteGlyphRangeFromTextBlocks
5  UIFoundation                   0x2384f89f8 -[NSLayoutManager setTextStorage:]
6  CoreFoundation                 0x22ddbe5dc -[__NSArrayM enumerateObjectsWithOptions:usingBlock:]
7  UIFoundation                   0x2384ef4ac -[NSTextStorage dealloc]
8  UIFoundation                   0x2384f4b90 -[NSConcreteTextStorage dealloc]
9  UIKitCore                      0x25b4ffd84 -[UITextView .cxx_destruct]
10 libobjc.A.dylib                0x22d0a8478 object_cxxDestructFromClass(objc_object*, objc_class*)
11 libobjc.A.dylib                0x22d0b871c objc_destructInstance
12 libobjc.A.dylib                0x22d0b8784 object_dispose
13 UIKitCore                      0x25b1f0fac -[UIResponder dealloc]
14 UIKitCore                      0x25b63fdcc -[UIView dealloc]
15 UIKitCore                      0x25b5ec9c8 -[UIScrollView dealloc]
16 UIKitCore                      0x25b4f08a0 -[UITextView dealloc]
17 WordPress                      0x102206b80 WPRichContentView.__deallocating_deinit (WPRichContentView.swift:159)
18 WordPress                      0x102206bbc @objc WPRichContentView.__deallocating_deinit (<compiler-generated>)
19 CoreFoundation                 0x22de370d4 cow_cleanup
20 CoreFoundation                 0x22ddbf408 -[__NSArrayM dealloc]
21 libobjc.A.dylib                0x22d0c338c (anonymous namespace)::AutoreleasePoolPage::pop(void*)
22 CoreFoundation                 0x22dee6a14 _CFAutoreleasePoolPop
23 CoreFoundation                 0x22de63cc4 __CFRunLoopRun
24 CoreFoundation                 0x22de631f0 CFRunLoopRunSpecific
25 GraphicsServices               0x2300dc584 GSEventRunModal
26 UIKitCore                      0x25b1c4c00 UIApplicationMain
27 WordPress                      0x102013a80 main (main.m:11)
28 libdyld.dylib                  0x22d922bb4 start

First Seen In: 11.4.1.1
Users Affected: 205
Source: 5c05f0baf8b88c29637135f2-fabric

Quick Analysis:
This bug appears to have been introduced in: https://github.com/wordpress-mobile/WordPress-iOS/commit/57b6cc0.

This code:
https://github.com/wordpress-mobile/WordPress-iOS/blob/57b6cc07f81d8563abdbc1f70d69c756decc0cd5/WordPress/Classes/ViewRelated/Views/WPRichText/WPRichContentView.swift#L162-L164

seems to be sometimes messing with the UITextView deallocation process in a way that's corrupting something.

I'm also not entirely certain that it's needed(?) – it seems that WPRichContentView is the sole owner of all of its RichMedia objects, which should mean that they'd automatically be deallocated properly when WPRichContentView is destroyed.

[designsimply]

~3.3 crashes per day in the last 90 days.
^{(5c05f0baf8b88c29637135f2-fabric-ios)}

This bug appears to have been introduced in: 57b6cc0.

@bummytime can you confirm?

[designsimply]

Never mind that last request. Decided to leave this in To Do for now because it's causing fewer crashes per day compared to other issues and will circle back to it to check on impact again in a few months or when the other crashes are resolved (whichever comes first). 🙂

[bummytime]

Thank you @designsimply — my apologies for not responding to your previous comment sooner.

Decided to leave this in To Do for now because it's causing fewer crashes per day compared to other issues and will circle back to it to check on impact again in a few months or when the other crashes are resolved (whichever comes first)

👍

[designsimply]

Not a problem because this is not a super high priority crash right now. If it were, I wouldn't push it back. 🙂 For a case like this, the reason I pinged was just in case the commit referenced sparked your memory and could be fixed quickly. If it can't then it just moves down in the list for now. 👍

[designsimply]

30-day impact: ~4 per day
Users affected: 82
First seen in: 11.4.1.1
Last seen in: 12.1.3

^{(5c05f0baf8b88c29637135f2-fabric-ios)}

[designsimply]

30-day impact: ~3 per day for WPiOS 12.2 and below
Users affected: 50
Last seen in: 12.2.0.3

^{(5c05f0baf8b88c29637135f2-fabric-ios)}

16-day impact: ~3 per day for WPiOS 12.3+
Users affected: 45
Last seen in: 12.3.1

^{(https://sentry.io/share/issue/d8febc329fe94104805ab63529949268/)}

[jtreanor]

There was a bunch of duplicate crashes being track for this in Sentry. I have merged them in into https://sentry.io/share/issue/d8febc329fe94104805ab63529949268/.

I'm doing a bit of digging and I'm starting to wonder if the change in https://github.com/wordpress-mobile/WordPress-iOS/commit/57b6cc0 are a red herring. The crash seems to occur as part of UITextView's internal deinit process. The best theory I can come up with is that the text view is being accessed from different threads (this might be something similar). I'll keep looking.

[jtreanor]

The best theory I can come up with is that the text view is being accessed from different threads (this might be something similar). I'll keep looking.

Okay, I think I have debunked this theory. WPRichContentView is never accessed off the main thread. I'm at a bit of a loss here so I'm going to move on to another issue.

[designsimply]

Just popping in with some updated stats:

90-day impact: ~2 per day
Users affected in the last 90 days: 99

^{https://sentry.io/share/issue/d8febc329fe94104805ab63529949268/}

[designsimply]

90-day impact: ~3 per day
Users affected in the last 90 days: 54

^{https://sentry.io/share/issue/d8febc329fe94104805ab63529949268/}

[aerych]

Dug into this a bit.

Here's what I'm seeing in the code:

• WPRichContentView is deallocated and its deinit is called.
• Each WPRichTextImage has its clean method called.
• The WPRichTextImage calls clean and prepForReuse on its internal CachedAnimatedImageView instance.
• The call to clean zeros out data, and cancels any inflight session tasks. Yay. (But 😱 we have a view responsible for networking 😬 ).
• The call to prepForReuse is a pass through and calls the internal Gifu prepareForReuse method, which ultimately sets a CADisplayLink instance's isPaused property to true.

Here's what I'm seeing in the stack trace:

Near the beginning we see an array is being dealloced.
Its not clear what the array contained, but we see that there were related references to WPRichContentView. (Perhaps a UIView's subviews?)
The core text stack is unwound until we reach the NSTextStorage where it seems to enumerate its NSLayoutManagers.
It looks like individual NSLayoutManagers appear to set their internal NSTextStorage references to nil (this is a hunch).
Finally, text blocks and storage runs of glyphs are cleaned up. Here we end up with an invalid index, and an invalid range size, and finally our crash.

There's no obvious intersection between the work being done in the deinit method and what we see in the stack trace. Whatever is happening is well hidden.

A theory: the range and index are invalid (the value of Int64.max actually) possibly due to the glyphs being nil--perhaps because they've already been released--tho I haven't found a code path where this would happen. I'm suspicious of the ReaderCommentsViewController being somehow involved. Its cells have their own WPRichTextContentView instances and it caches NSAttributedStrings as a performance strategy.

All that said, let's try adjusting the deinit behavior since its the best lead we currently have, and see if it makes any difference. https://github.com/wordpress-mobile/WordPress-iOS/pull/12427

[sentry-io[bot]]

Sentry Issue: WORDPRESS-IOS-2S6T

NSRangeException: NSMutableRLEArray objectAtIndex:effectiveRange:: Out of bounds
  File "WPRichContentView.swift", line 611, in WPRichContentView.gestureRecognizer
  File "<compiler-generated>", in WPRichContentView.gestureRecognizer
  File "<compiler-generated>", in NSMutableAttributedString.init
  File "<compiler-generated>", in NSMutableAttributedString.init
  File "<compiler-generated>", in NSMutableAttributedString.__allocating_init

Events in the last 90 days: 1,100
Users affected in the last 90 days: 127
_{https://sentry.io/share/issue/023bf76978934bc49fc2da5c6241e621/}

[designsimply]

I searched Sentry for "NSMutableRLEArray objectAtIndex:effectiveRange:: Out of bounds" and found 188 crashes were recorded for 14.3.0.3 since it was released 17d ago on 2020-03-09.

[designsimply]

Sentry Issue: WORDPRESS-IOS-2S6T
Events in 14.3.0.3 since it was released 18d ago on Mar 9: 186

[designsimply]

Events in the last 90d: 1600
Events in the last 11d: zero since 14.9 was released on 2020-06-01
Users affected in the last 90d: 347
_{https://sentry.io/share/issue/023bf76978934bc49fc2da5c6241e621/}

Closing because the 90d graph for this issue appears to have dropped off significantly after Apr 20.

^{(internal reference: pbArwn-uu-p2 is also a relevant discussion)}