This is Xamarin's bug tracking system. For product support, please use the support links listed in your Xamarin Account.
Bug 41035 - DataViewTest2.DataView_ListChangedEventTest occasionally fails with llvm+sgen
Summary: DataViewTest2.DataView_ListChangedEventTest occasionally fails with llvm+sgen
Status: VERIFIED FIXED
Alias: None
Product: iOS
Classification: Xamarin
Component: BCL Class Libraries (show other bugs)
Version: XI 9.8 (tvOS / C7)
Hardware: PC Mac OS
: --- normal
Target Milestone: 10.0.0 (C8)
Assignee: Zoltan Varga
URL:
Depends on:
Blocks:
 
Reported: 2016-05-11 19:28 UTC by Alexander Köplinger [MSFT]
Modified: 2016-08-10 16:41 UTC (History)
5 users (show)

See Also:
Tags:
Is this bug a regression?: ---
Last known good build:


Attachments

Description Alexander Köplinger [MSFT] 2016-05-11 19:28:33 UTC
(this was split from https://bugzilla.xamarin.com/show_bug.cgi?id=32583)

I can repro the DataViewTest2.DataView_ListChangedEventTest failure on my iPad mini 4 (iOS 9.1) with Xamarin.iOS 9.8.0.294/6950f7b in about 1 of 20 runs.

I used the System.Data tests with the Release+llvm+sgen configuration.

The key thing for me was switching to SGen instead of Boehm, with Boehm or without LLVM (thumb didn't matter in my testing) I couldn't get it to repro.

I saw two different failures:

> [FAIL] DataViewTest2.DataView_ListChangedEventTest :   #10
>   Expected: -1
>   But was:  0
> 		  at MonoTests.System.Data.DataViewTest2.DataView_ListChangedEventTest () <0x1fa9e1 + 0x0023a> in <filename unknown>:0 


> [FAIL] DataViewTest2.DataView_ListChangedEventTest : System.NullReferenceException : Object reference not set to an instance of an object
> 	  at MonoTests.System.Data.DataViewTest2.DataView_ListChangedEventTest () <0x1d29e1 + 0x001cb> in <filename unknown>:0 


This seems to be happening for quite some time already, in #32583 it was noted it happens in C6 too.
It's not clear yet where the bug is, switching the GC to SGen might just cause it to manifest now without being an actual GC bug.
Comment 1 GouriKumari 2016-07-01 14:43:16 UTC
Observed the failure on two different devices an iPhone6Plus and iPad2

Error:
[FAIL] DataViewTest2.DataView_ListChangedEventTest :   #10
  Expected: -1
  But was:  0

##Logs:
Test Log:
https://wrench.internalx.com/Wrench/WebServices/Download.aspx?workfile_id=15597838
https://wrench.internalx.com/Wrench/WebServices/Download.aspx?workfile_id=15552391

Build Log:
https://gist.github.com/GouriKumari/cf09f8ca26ab5bb794f125060b6e672c
https://gist.github.com/GouriKumari/db21c76797f8a40ee192c29f063f7d06

##Test Env:
mtouch 9.8.2.10 (cycle7: 843a527)
mtouch 9.8.2.7 (cycle7: 08215a3)
Comment 2 Rodrigo Kumpera 2016-07-01 15:05:09 UTC
Hey Zoltan,

Please take a look at this one.
Comment 3 Zoltan Varga 2016-07-03 00:01:37 UTC
This happens because DataViewListener keeps a weak ref to the DataView, and deletes the listener if the DataView object gets gc-d (in DataViewListener.MaintainDataView). This can happen in the test:

			DataTable table = new DataTable ("test");
			table.Columns.Add ("col1", typeof(int));
			
			DataView view = new DataView (table);
			
			view.ListChanged += new ListChangedEventHandler (dv_ListChanged);
			
			evProp = null;
			table.Rows.Add (new object[] {1});
			Assert.AreEqual (0, evProp.NewIndex, "#1");

By the time the Add method is called, the DataView object might be dead, so the callback is not called, causing evProp to stay null, leading to a nullref.
Comment 4 Rodrigo Kumpera 2016-07-04 19:36:01 UTC
Hey Zoltan,

So can we say that the problem in this case is with the test?

Could we fix it by using a GC.KeepAlive to ensure DataView will remain alive until the end of the test?
Comment 5 Zoltan Varga 2016-07-04 20:32:54 UTC
We can certainly fix the test, whenever the current behaviour is intended or not is another question.
Comment 6 Zoltan Varga 2016-07-05 20:37:56 UTC
Fixed in master/4.5.1 branch.
Comment 7 GouriKumari 2016-07-07 14:38:09 UTC
@vargaz: Is this fix applied to xamarin-macios master repo.  If this is a test only fix , I think we can move it to Cycle8. Could you please confirm.
Comment 8 Zoltan Varga 2016-07-07 19:34:36 UTC
Mono was updated but xamarin-macios was not bumped to use it yet.
Comment 9 GouriKumari 2016-07-14 16:55:58 UTC
@vargaz: Can you update this bug if the fix is in Cycle8 branch? Since cycle8 is  following mono4.6.0, will it be included already?
Comment 10 Zoltan Varga 2016-07-14 17:15:02 UTC
The fix is in cycle8.
Comment 12 GouriKumari 2016-08-10 16:41:28 UTC
Verified with 64 bit device. DataViewTest2.DataView_ListChangedEventTest pass on devices.

##Logs:
https://wrench.internalx.com/Wrench/GetFile.aspx?id=16104398
https://wrench.internalx.com/Wrench/GetFile.aspx?id=16104402
https://wrench.internalx.com/Wrench/GetFile.aspx?id=16104406

## Test Env:
mtouch 9.10.0.52 (cycle8: fc97af6)

## Devices:
iPadAir, iPad2 and iPhone6SPlus

Note You need to log in before you can comment on or make changes to this bug.