-
Notifications
You must be signed in to change notification settings - Fork 515
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
[iOS13] NSIndexPath Dispose Clarification #7206
Comments
Some quick (not executed the test case yet), semi-related notes:
What you want to look is more likely |
I do have a theory (basically the caching I mentioned earlier) but the attached sample does not crash for me (but it does for someone else on the team). Just to be sure we see the exact same thing, can you copy the full abort message (it's generally followed by a lot of things) and also attach the crash report to the issue ? thanks! |
It still does not crash (for me) but both |
Looks similar to https://bugzilla.xamarin.com/show_bug.cgi?id=25511 Adding Console.WriteLine ($"ReferenceEquals: {Object.ReferenceEquals (indexPath, anotherIndexPath)}"); prints
So the value is identical and iOS (like only 13+ to be confirmed) return a cached value. Many API, like
Since Xamarin.iOS already know that handle it assume it's the same instance and returns it. That's normal since the ObjC code could be Anyhow the problem manifest itself when your code proceed to dispose it. The
It's generally a good advice but:
[1] I realize that, due to caching, it's not easy/foolproof to know if you can dispose of something :(
|
Thanks for looking into this. I've looked into your comments this morning.
I can confirm that that following code operates the same on iOS 10 -> 13. var ip1 = NSIndexPath.FromRowSection(0, 0); //handle: AAAAA
var ip2 = NSIndexPath.FromRowSection(0, 0); //handle: AAAAA
var ip3 = NSIndexPath.FromRowSection(1, 0); //handle: ZZZZZ
var s1 = new NSString(string.Empty); //handle: BBBBB
var s2 = new NSString(string.Empty); //handle: BBBBB
var s3 = new NSString(string.Empty); //handle: YYYYY ip1 & ip2 have the same handles. ip3 has a different handle. This makes sense to me and I can understand the reason that s1 / ip1 and s2 / ip2 can share a handle. if I then run the following: s.1Dispose();
// s1 Handle:
// s2 Handle: BBBBB
// s3 Handle: ZZZZZ
ip1.Dispose();
// ip1 Handle:
// ip2 Handle:
// ip3 Handle: ZZZZZ I don't understand the result for this. For the For the
In the snippet below I would say I am disposing of the index path after it has been used. Unfortunately it has the side effect of disposing the index path passed into the function. We then pass this index path to the call to Apple seem to have tighted up this API call in iOS13 causing the application to assert on a /* 'indexPath' surfaced to us from Xamarin by the Obj-C runtime requesting a cell
* from the table view. */
public void GetItemAt(NSIndexPath indexPath)
{
int rowIndex = GetRowIndexFromIndexPath(indexPath);
// 'anotherIndexPath is created for use...
using (var anotherIndexPath = NSIndexPath.FromRowSection(rowIndex, 0))
{
return base.GetItemAt(anotherIndexPath);
}
/* 'anotherIndexPath' has been used and so we dispose it. We still want to
* use 'indexPath' but the act of disposing THIS index path instance also
* disposes the one given to us from Xamarin. */
} A similar example in Swift does not show this. Should swift do the same thing, I'd expect the second lot of memory addresses printed out to have ip1 as weak var ip1 = NSIndexPath(row: 1, section: 1);
var ip2 = NSIndexPath(row: 1, section: 1);
var ip3 = NSIndexPath(row: 12, section: 1);
let addr_ip1_1 = Unmanaged.passUnretained(ip1!).toOpaque();
let addr_ip2_1 = Unmanaged.passUnretained(ip2).toOpaque();
let addr_ip3_1 = Unmanaged.passUnretained(ip3).toOpaque();
debugPrint("ip1 Handle: \(addr_ip1_1)");
debugPrint("ip2 Handle: \(addr_ip2_1)");
debugPrint("ip3 Handle: \(addr_ip3_1)");
debugPrint("> Mock GetItemAt...");
indexPathVoodoo(ip: &ip1!);
debugPrint("< Mock GetItemAt...");
let addr_ip1_2 = Unmanaged.passUnretained(ip1!).toOpaque();
let addr_ip2_2 = Unmanaged.passUnretained(ip2).toOpaque();
let addr_ip3_2 = Unmanaged.passUnretained(ip3).toOpaque();
debugPrint("ip1 Handle: \(addr_ip1_2)");
debugPrint("ip2 Handle: \(addr_ip2_2)");
debugPrint("ip3 Handle: \(addr_ip3_2)");
debugPrint("Done.");
func indexPathVoodoo(ip : inout NSIndexPath)
{
let another_ip = NSIndexPath(row: ip.row, section: 1);
let addr_ip = Unmanaged.passUnretained(ip).toOpaque();
let addr_aip = Unmanaged.passUnretained(another_ip).toOpaque();
debugPrint(" ip handle: \(addr_ip)");
debugPrint("aip handle: \(addr_aip)");
// another_ip goes out of scope so Swift will clean it up
} With output: "ip1 Handle: AAAAA"
"ip2 Handle: AAAAA"
"ip3 Handle: ZZZZZ"
"> Mock GetItemAt..."
" ip handle: AAAAA"
"aip handle: AAAAA"
"< Mock GetItemAt..."
"ip1 Handle: AAAAA"
"ip2 Handle: AAAAA"
"ip3 Handle: ZZZZZ"
I'd have suspected any API that does this would have it explicitly mentioned in the Apple documentation, such as https://developer.apple.com/documentation/uikit/uiimage/1624146-imagenamed?language=objc
The exception reported to us by our crash reporting system: Foundation.MonoTouchException: Objective-C exception thrown. Name: NSInternalInconsistencyException Reason: Attempted to dequeue a cell for a nil index path We've had customer reports of this from the following iOS verisons:
Output from Visual Studio Mac:
|
Typo for
? you have a different handle so it's likely not The difference there is that your code is doing a A call to Think of native code doing
If you call Xamarin.iOS does not know how each native method works, but if the same handle is returned then we use the existing managed instance. If not then we create a new one. |
Yes, indeed. Sorry about that...
That makes a lot of sense. Thanks again for looking into this and providing some clarification :) |
Previous advice I've recieved from the Xamarin team is to dispose of Xamarin objects as soon as possible after use. I believe it will sever the connections into the Obj-C land and reduces the work required by GC and the Obj-C runtime.
Our app has quite a number of table and collection views which have many sections and are backed by a single collection. We do a fair bit of work to translate
NSIndexPath
Row/Section to an index path in the form CollectionIndex/0.We have a number of table and collection view helper classes but the rough code follows something like the following:
This code was working fine until the release of iOS 13. Whenever this runs the line marked
//IssueOccurs
asserts with the messageUITableView dequeue cell with 'nil' index path
.On further investigation it shows that any index path created regardless of of Row/Section provided will have the same underlying
ClassHandle
property. Therefore, any index path that is disposed, disposes of the underlying index path class.With iOS13, Apple appear to have tightened up the
UITableView
API to assert when given a disposed index path.If the recommended way is to dispose of Xamarin objects is there a bug in how Xamarin are surfacing
NSIndexPath
indexes?I'd have expected that each index path would have a different class handle. It makes sense to me you might recieve the same backing class handle when requesting an index path with the same row/section value in the same scope. It surprises me that all index paths are backed by the same handle regardless of row/section value.
In this case, if I dispose of
actualIndexPath
I would expect that only that index path is disposed and the original index path would remain untouched.Sample Application (Zip below)
I've attached a sample application annotated and demonstrating what I've seen.
If you run the app on iOS 12 or earlier, the app should run showing a TableView with 1 (empty) cell.
If you run the app on iOS 13 the app will assert and crash before the view appears. If you remove the
using
statement around the construction of the seperate index path everything works as I would expect.Note:
UICollectionView
doesn't assert when given a disposed index path in the same wayUITableView
does.Environment
Example Project (If Possible)
NilIndexPath.zip
The text was updated successfully, but these errors were encountered: