dotnet / windows-sdk-for-google-analytics

SDK to connect to Google Analytics from Windows Store (UWP) Apps, Windows desktop apps written with .NET, and Xamarin Apps
MIT License
102 stars 33 forks source link

BUG. hits object isn't locked in DispatchQueuedHits() #33

Open 23W opened 6 years ago

23W commented 6 years ago

Current source of ServiceManager class contains method:

    async Task DispatchQueuedHits(IEnumerable<Hit> hits)
    {
        using (var httpClient = GetHttpClient())
        {
            var now = DateTimeOffset.UtcNow;
            foreach (var hit in hits)
            {
                if (isEnabled && (!ThrottlingEnabled || hitTokenBucket.Consume()))
                {
                    // clone the data
                    var hitData = hit.Data.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
                    hitData.Add("qt", ((long)now.Subtract(hit.TimeStamp).TotalMilliseconds).ToString());
                    await DispatchHitData(hit, httpClient, hitData);
                }
                else
                {
                    lock (hits) // add back to queue
                    {
                        this.hits.Enqueue(hit);
                    }
                }
            }
        }
    }

where lock statement locks incorrect object, instead of lock (hits) it should be lock (this.hits). So correct code should be:

    async Task DispatchQueuedHits(IEnumerable<Hit> hits)
    {
        using (var httpClient = GetHttpClient())
        {
            var now = DateTimeOffset.UtcNow;
            foreach (var hit in hits)
            {
                if (isEnabled && (!ThrottlingEnabled || hitTokenBucket.Consume()))
                {
                    // clone the data
                    var hitData = hit.Data.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
                    hitData.Add("qt", ((long)now.Subtract(hit.TimeStamp).TotalMilliseconds).ToString());
                    await DispatchHitData(hit, httpClient, hitData);
                }
                else
                {
                    lock (this.hits) // add back to queue
                    {
                        this.hits.Enqueue(hit);
                    }
                }
            }
        }
    }