dotnet/windows-sdk-for-google-analytics

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

23W opened this issue · 0 comments

23W commented

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);
                    }
                }
            }
        }
    }