Bug 41035 - DataViewTest2.DataView_ListChangedEventTest occasionally fails with llvm+sgen
Summary: DataViewTest2.DataView_ListChangedEventTest occasionally fails with llvm+sgen
Alias: None
Product: iOS
Classification: Xamarin
Component: BCL Class Libraries ()
Version: XI 9.8 (tvOS / C7)
Hardware: PC Mac OS
: --- normal
Target Milestone: 10.0.0 (C8)
Assignee: Zoltan Varga
Depends on:
Reported: 2016-05-11 19:28 UTC by Alexander Köplinger [MSFT]
Modified: 2016-08-10 16:41 UTC (History)
5 users (show)

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

Notice (2018-05-24): bugzilla.xamarin.com is now in read-only mode.

Please join us on Visual Studio Developer Community and in the Xamarin and Mono organizations on GitHub to continue tracking issues. Bugzilla will remain available for reference in read-only mode. We will continue to work on open Bugzilla bugs, copy them to the new locations as needed for follow-up, and add the new items under Related Links.

Our sincere thanks to everyone who has contributed on this bug tracker over the years. Thanks also for your understanding as we make these adjustments and improvements for the future.

Please create a new report on Developer Community or GitHub with your current version information, steps to reproduce, and relevant error messages or log files if you are hitting an issue that looks similar to this resolved bug and you do not yet see a matching new report.

Related Links:

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 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

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

Test Log:

Build Log:

##Test Env:
mtouch (cycle7: 843a527)
mtouch (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.


## Test Env:
mtouch (cycle8: fc97af6)

## Devices:
iPadAir, iPad2 and iPhone6SPlus