Runtime: [NetFX compat]: EnumerableQuery<T>((IEnumerable<T>)null).GetEnumerator() difference in behavior

[safern]

The following code has a different behavior in NetFX:

IQueryable<T> query = new EnumerableQuery<T>((IEnumerable<T>)null);
var enumerator = query.GetEnumerator();

If we call GetEnumerator on an IQueryable<T> instance that has a null IEnumerable<T> in CoreFX we would throw an InvalidOperationException, but in NetFX as stated in the MSDN Documentation it will not throw:

Returns an enumerator that can iterate through the associated IEnumerable<T> collection, or, if it is null, through the collection that results from rewriting the associated expression tree as a query on an IEnumerable<T> data source and executing it.

I've tried to execute the sample code I included before in a .NET 4.6.2 Console Application and it will cause an StackOverflowException.

Also this is causing in one of our System.Linq.Queryable tests will cause it to hang forever. Test source code

cc: @tarekgh @danmosemsft

[karelz]

@safern do you recommend to change .NET Core or Desktop?

[tarekgh]

do you recommend to change .NET Core or Desktop?

The owner of System.Linq need to decide about that. I would say I like corefx behavior more than the desktop.

[karelz]

OK, pushing the decision to 2.0 then ... @VSadov @OmarTawfik can you please take a look?

[safern]

I like corefx behavior more as well, I rather have it throwing an InvalidOperationException than causing and StackOverflow.

[JonHanna]

I like corefx behavior more as well, I rather have it throwing an InvalidOperationException than causing and StackOverflow.

Me too, that's why dotnet/corefx#3547 introduced that change :smile:

I'd actually prefer if new EnumerableQuery<T>((IEnumerable<T>)null) threw ArgumentNullException but since it's possible to fruitfully use such an object as an IQueryProvider that could have broken working code.

but in NetFX as stated in the MSDN Documentation it will not throw:

The documentation isn't quite incorrect. If you create an EnumerableQuery<T> from an expression then the IEnumerable<T> set by the other constructor is null, and it will do exactly that. Pass null though and you get an expression that wraps the EnumerableQuery itself, so rewriting and compiling that results in a null enumerable, which results in the expression being rewritten and compiled…

[ViktorHofer]

@VSadov @OmarTawfik Have you had time to look into this? 2.0 milestone is getting closer.

[OmarTawfik]

I also prefer corefx's approach, and changing the desktop. @VSadov do you agree?

[JonHanna]

I opened the issue and submitted the change after losing some time trying to find a bug that had hit the stack-overflow case in a desktop project, so I definitely favour changing desktop 😄

[VSadov]

Lets fix in Desktop.
The behavior in CoreFx is indeed better and noone could take dependency on stackoverflow.

[danmosemsft]

Good catch @safern !

[ViktorHofer]

How do we normally raise an issue in netfx? Create an issue there in tfs and close this one?

[safern]

By adding the netfx-port-consider label it opens automatically an issue in TFS. So this one should be already opened there. Closing it here.