NullReferenceException in OData client Graph.UnreachableVertices()

[lezi226]

Hi, we hit a 100% repro NullReferenceException in OData client.

It’s our code that trigger the crash:

Guid[] inputUsers = { user1.Id, user2, user3.Id }; //user1 and user3 exist, but user2 doesn’t exist.

// user1 exist, so the first iteration of the loop execute without any issue.
// user2 doesn’t exist, will trigger HttpStatusCode.NotFound exception, and will be caught by the catch() block, and our foreach() loop contine.
// user3 exist, then the container.SaveChanges() will cause the NullReferenceException.
                foreach (var userId in inputUsers)
                {
                    Guid id = userId;
                    User user = null;
                    try
                    {
                        var query = container.Users.Expand(u => u.Property1).Where(u => u.Id == id);

                        var collection = new DataServiceCollection<User>(query); // since user2 doesn’t exist, this line of code will trigger exception which will be caught by below catch(), and the foreach loop will continue to the next user3. 
// the special thing here is a new DataServiceCollection is created to wrap query variable.
                        ////var collection = query; 
                        user = collection.Single();
                    }
                    catch (DataServiceQueryException dsqe)
                    {
                        var dsce = dsqe.InnerException as DataServiceClientException;

                        if (dsce != null && dsce.StatusCode == (int)HttpStatusCode.NotFound)
                        {
                            continue;
                        }

                        throw;
                    }

                    container.DeleteObject(user.Property1);
                    container.SaveChanges(); // when executes to user3, this line cause the NullReferenceException.

                    user.Property1Action(true, true);
                }

It’s the callstack of the crash:

>            Microsoft.OData.Client.dll!Microsoft.OData.Client.BindingGraph.Graph.UnreachableVertices() Line 896 + 0x14 bytes          C#
Microsoft.OData.Client.dll!Microsoft.OData.Client.BindingGraph.Graph.RemoveUnreachableVertices(System.Action<object> detachAction) Line 868 + 0x9 bytes          C#
                Microsoft.OData.Client.dll!Microsoft.OData.Client.BindingGraph.RemoveUnreachableVertices() Line 444                C#
                Microsoft.OData.Client.dll!Microsoft.OData.Client.BindingGraph.RemoveNonTrackedEntities() Line 415                C#
                Microsoft.OData.Client.dll!Microsoft.OData.Client.BindingObserver.OnChangesSaved(object sender, Microsoft.OData.Client.SaveChangesEventArgs eventArgs) Line 1030     C#
Microsoft.OData.Client.dll!Microsoft.OData.Client.DataServiceContext.SaveChanges(Microsoft.OData.Client.SaveChangesOptions options) Line 1691           C#
                Microsoft.OData.Client.dll!Microsoft.OData.Client.DataServiceContext.SaveChanges() Line 1661 + 0x1e bytes     C#

It’s the code that trigger the crash in OData client:

D:\tmp\OData\odata.net-6.8.1\odata.net-6.8.1\src\Client\Microsoft\OData\Client\Binding\BindingGraph.cs
            private IEnumerable<Vertex> UnreachableVertices()
            {
                Queue<Vertex> q = new Queue<Vertex>();

                this.Root.Color = VertexColor.Gray; // here this.Root is null, and caused the crash

I debugged a little bit, and found the root cause of the issue:
When doing var collection = new DataServiceCollection<User>(query); for not existed user2, in D:\tmp\OData\odata.net-6.8.1\odata.net-6.8.1\src\Client\Microsoft\OData\Client\Binding\DataServiceCollectionOfT.cs
below code is executed, but will raise the HttpStatusCode.NotFound exception

        private void InternalLoadCollection(IEnumerable<T> items)
        {
            Debug.Assert(items != null, "items != null");
#if !ASTORIA_LIGHT && !PORTABLELIB
            // For SDP, we must execute the Query implicitly
            DataServiceQuery<T> query = items as DataServiceQuery<T>;
            if (query != null)
            {
                items = query.Execute() as QueryOperationResponse<T>;
            }

Callstack:

> Microsoft.OData.Client.dll!Microsoft.OData.Client.DataServiceCollection<MyODataClient.User>.InternalLoadCollection(System.Collections.Generic.IEnumerable<MyODataClient.User> items) Line 643 + 0xa bytes          C#
Microsoft.OData.Client.dll!Microsoft.OData.Client.DataServiceCollection<MyODataClient.User>.StartTracking(Microsoft.OData.Client.DataServiceContext context, System.Collections.Generic.IEnumerable<MyODataClient.User> items, string entitySet, System.Func<Microsoft.OData.Client.EntityChangedParams,bool> entityChanged, System.Func<Microsoft.OData.Client.EntityCollectionChangedParams,bool> collectionChanged) Line 734 + 0xc bytes             C#
Microsoft.OData.Client.dll!Microsoft.OData.Client.DataServiceCollection<MyODataClient.User>.DataServiceCollection(Microsoft.OData.Client.DataServiceContext context, System.Collections.Generic.IEnumerable<MyODataClient.User> items, Microsoft.OData.Client.TrackingMode trackingMode, string entitySetName, System.Func<Microsoft.OData.Client.EntityChangedParams,bool> entityChangedCallback, System.Func<Microsoft.OData.Client.EntityCollectionChangedParams,bool> collectionChangedCallback) Line 185 + 0x18 bytes               C#
Microsoft.OData.Client.dll!Microsoft.OData.Client.DataServiceCollection<MyODataClient.User>.DataServiceCollection(System.Collections.Generic.IEnumerable<MyODataClient.User> items) Line 87    C#

At this time, BindingObserver of DataServiceCollection is allocated, but its BindingGraph.Root isn’t set. Then when executing query for user3, and raise event changes inside of SaveChanges(), the BindingObserver. OnChangesSaved() will be invoked for BindingObserver created for user2, and then crash since BindingGraph.Root is null.

I think for the fix, we should either invoke StopTracking() when hitting exception in DataServiceCollection.InternalLoadCollection(), or check BindingGraph.Root before access it.

Is this a known issue? Not a high priority issue though since we can replace “var collection = new DataServiceCollection(query);” with “var response = query.Execute(); var collection = new DataServiceCollection(response);
but still want to let you guys know the issue since it may impact other scenarios.

Thanks

[lewischeng-ms]

Hi @dengxi , I think your description of the issue and its cause is helpful. Thanks! Now we think it better to check BindingGraph.Root before traversal rather than calling BindingObserver.StopTracking() because it may prevent further entities loaded by DataServiceCollection<T>.Load() from being tracked.

[lewischeng-ms]

Checked in as 1701387

[lezi226]

Hi, thanks for fixing it.
Date: Mon, 30 Mar 2015 23:43:51 -0700
From: notifications@github.com
To: odata.net@noreply.github.com
CC: dengkuixi@outlook.com
Subject: Re: [odata.net] NullReferenceException in OData client Graph.UnreachableVertices() (#107)

Hi @dengxi , I think your description of the issue and its cause is helpful. Thanks! Now we think it better to check BindingGraph.Root before traversal rather than calling BindingObserver.StopTracking() because it may prevent further entities loaded by DataServiceCollection.Load() from being tracked.

—
Reply to this email directly or view it on GitHub.