Skip to content

Commit

Permalink
Update queue get/create/update logic in HttpSysDelegator.AddOrUpdateR…
Browse files Browse the repository at this point in the history
…ules to avoid GC timing bug (#2487)
  • Loading branch information
NGloreous committed May 3, 2024
1 parent b8cc3f3 commit 3c0d3f7
Showing 1 changed file with 19 additions and 35 deletions.
54 changes: 19 additions & 35 deletions src/ReverseProxy/Delegation/HttpSysDelegator.cs
Expand Up @@ -164,48 +164,32 @@ private void AddOrUpdateRules(ClusterState cluster)
var queueKey = new QueueKey(queueName, urlPrefix);
if (!_queuesPerDestination.TryGetValue(destination, out var queue) || !queue.Equals(queueKey))
{
if (!_queues.TryGetValue(queueKey, out var queueWeakRef) || !queueWeakRef.TryGetTarget(out queue))
var queueWeakRef = _queues.GetOrAdd(queueKey, key => new WeakReference<DelegationQueue>(new DelegationQueue(key.QueueName, key.UrlPrefix)));
if (!queueWeakRef.TryGetTarget(out queue))
{
// Either the queue hasn't been created or it has been cleaned up.
// Create a new one, and try to add it if someone else didn't beat us to it.
queue = new DelegationQueue(queueName, urlPrefix);
queueWeakRef = new WeakReference<DelegationQueue>(queue);
queueWeakRef = _queues.AddOrUpdate(
queueKey,
(key, newValue) => newValue,
(key, value, newValue) => value.TryGetTarget(out _) ? value : newValue,
queueWeakRef);
queueWeakRef.TryGetTarget(out queue);
}

if (queue is not null)
{
// We call this outside of the above if bock so that if previous
// initialization failed, we will retry it for every new destination added.
var queueState = queue.Initialize(_serverDelegationFeature);
if (!queueState.IsInitialized)
// The queue was GC'd since it was originally created
lock (queueWeakRef)
{
Log.QueueInitFailed(
_logger,
destination.DestinationId,
queueName,
urlPrefix,
queueState.InitializationException);
if (!queueWeakRef.TryGetTarget(out queue))
{
queue = new DelegationQueue(queueName, urlPrefix);
queueWeakRef.SetTarget(queue);
}
}

_queuesPerDestination.AddOrUpdate(destination, queue);
}
else

var queueState = queue.Initialize(_serverDelegationFeature);
if (!queueState.IsInitialized)
{
// This should never happen because we always create a new one above
_queuesPerDestination.Remove(destination);
Log.QueueInitFailed(
_logger,
destination.DestinationId,
queueName,
urlPrefix,
new Exception("Delegation queue is null after adding a new one. This shouldn't happen."));
_logger,
destination.DestinationId,
queueName,
urlPrefix,
queueState.InitializationException);
}

_queuesPerDestination.AddOrUpdate(destination, queue);
}
}
else
Expand Down

0 comments on commit 3c0d3f7

Please sign in to comment.