Skip to content

Commit

Permalink
Revert change to make RequestRefresh run in the background
Browse files Browse the repository at this point in the history
RequestRefresh is a sync api, it is expected that the operation be done
when the method returns. With RequestRefresh being on a background thread, callers
can experience unexpected behavior.

Non blocking RequestRefresh should be done with issue 3040
  • Loading branch information
Keegan Caruso committed Jan 8, 2025
1 parent ead4201 commit 2f6a41d
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ public override void RequestRefresh()
_isFirstRefreshRequest = false;
if (Interlocked.CompareExchange(ref _configurationRetrieverState, ConfigurationRetrieverRunning, ConfigurationRetrieverIdle) == ConfigurationRetrieverIdle)
{
_ = Task.Run(UpdateCurrentConfiguration, CancellationToken.None);
UpdateCurrentConfiguration();
_lastRequestRefresh = now;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ public async Task VerifyInterlockGuardForRequestRefresh()
// Interlocked guard will block.
// Configuration should be AADCommonV1Config
signalEvent.Reset();
configurationManager.RequestRefresh();
_ = Task.Run(() => configurationManager.RequestRefresh());

// InMemoryDocumentRetrieverWithEvents will signal when it is OK to change the MetadataAddress
// otherwise, it may be the case that the MetadataAddress is changed before the previous Task has finished.
Expand All @@ -239,7 +239,7 @@ public async Task VerifyInterlockGuardForRequestRefresh()
// AADCommonV1Json would have been passed to the the previous retriever, which is blocked on an event.
configurationManager.MetadataAddress = "AADCommonV2Json";
TestUtilities.SetField(configurationManager, "_lastRequestRefresh", DateTimeOffset.MinValue);
configurationManager.RequestRefresh();
_ = Task.Run(() => configurationManager.RequestRefresh());

// Set the event to release the lock and let the previous retriever finish.
waitEvent.Set();
Expand Down Expand Up @@ -658,14 +658,13 @@ public async Task GetConfigurationAsync()
var configuration = await configManager.GetConfigurationAsync(CancellationToken.None);

TestUtilities.SetField(configManager, "_lastRequestRefresh", DateTimeOffset.UtcNow - TimeSpan.FromHours(1));
configManager.RequestRefresh();
configManager.MetadataAddress = "http://127.0.0.1";
configManager.RequestRefresh();
var configuration2 = await configManager.GetConfigurationAsync(CancellationToken.None);
IdentityComparer.AreEqual(configuration, configuration2, context);
if (!object.ReferenceEquals(configuration, configuration2))
context.Diffs.Add("!object.ReferenceEquals(configuration, configuration2)");


// get configuration from http address, should throw
// get configuration with unsuccessful HTTP response status code
TestUtilities.AssertFailIfErrors(context);
Expand Down

0 comments on commit 2f6a41d

Please sign in to comment.