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