[PM-1576] Fix Race condition AccountsManager registration (#2434)

* PM-1576 Moved registration of AccountsManager to avoid race conditions with the app start. To do so, added ConditionedAwaiterManager so that it handles a task to be awaited or completed depending on the callers.

* PM-1576 Fix format

* PM-1576 Fix throw to preserve StackTrace
This commit is contained in:
Federico Maccaroni 2023-04-07 18:24:54 +01:00 committed by GitHub
parent e5ce1760a6
commit 1823efa0e5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 132 additions and 37 deletions

View file

@ -81,7 +81,8 @@ namespace Bit.Droid
ServiceContainer.Resolve<IAuthService>("authService"),
ServiceContainer.Resolve<ILogger>("logger"),
ServiceContainer.Resolve<IMessagingService>("messagingService"),
ServiceContainer.Resolve<IWatchDeviceService>());
ServiceContainer.Resolve<IWatchDeviceService>(),
ServiceContainer.Resolve<IConditionedAwaiterManager>());
ServiceContainer.Register<IAccountsManager>("accountsManager", accountsManager);
}
#if !FDROID

View file

@ -22,6 +22,7 @@ namespace Bit.App.Utilities.AccountManagement
private readonly ILogger _logger;
private readonly IMessagingService _messagingService;
private readonly IWatchDeviceService _watchDeviceService;
private readonly IConditionedAwaiterManager _conditionedAwaiterManager;
Func<AppOptions> _getOptionsFunc;
private IAccountsManagerHost _accountsManagerHost;
@ -34,7 +35,8 @@ namespace Bit.App.Utilities.AccountManagement
IAuthService authService,
ILogger logger,
IMessagingService messagingService,
IWatchDeviceService watchDeviceService)
IWatchDeviceService watchDeviceService,
IConditionedAwaiterManager conditionedAwaiterManager)
{
_broadcasterService = broadcasterService;
_vaultTimeoutService = vaultTimeoutService;
@ -45,6 +47,7 @@ namespace Bit.App.Utilities.AccountManagement
_logger = logger;
_messagingService = messagingService;
_watchDeviceService = watchDeviceService;
_conditionedAwaiterManager = conditionedAwaiterManager;
}
private AppOptions Options => _getOptionsFunc?.Invoke() ?? new AppOptions { IosExtension = true };
@ -59,6 +62,8 @@ namespace Bit.App.Utilities.AccountManagement
public async Task StartDefaultNavigationFlowAsync(Action<AppOptions> appOptionsAction)
{
await _conditionedAwaiterManager.GetAwaiterForPrecondition(AwaiterPrecondition.EnvironmentUrlsInited);
appOptionsAction(Options);
await NavigateOnAccountChangeAsync();
@ -66,6 +71,8 @@ namespace Bit.App.Utilities.AccountManagement
public async Task NavigateOnAccountChangeAsync(bool? isAuthed = null)
{
await _conditionedAwaiterManager.GetAwaiterForPrecondition(AwaiterPrecondition.EnvironmentUrlsInited);
// TODO: this could be improved by doing chain of responsability pattern
// but for now it may be an overkill, if logic gets more complex consider refactoring it
@ -132,6 +139,8 @@ namespace Bit.App.Utilities.AccountManagement
{
try
{
await _conditionedAwaiterManager.GetAwaiterForPrecondition(AwaiterPrecondition.EnvironmentUrlsInited);
switch (message.Command)
{
case AccountsManagerMessageCommands.LOCKED:
@ -206,6 +215,8 @@ namespace Bit.App.Utilities.AccountManagement
public async Task LogOutAsync(string userId, bool userInitiated, bool expired)
{
await _conditionedAwaiterManager.GetAwaiterForPrecondition(AwaiterPrecondition.EnvironmentUrlsInited);
await AppHelpers.LogOutAsync(userId, userInitiated);
await NavigateOnAccountChangeAsync();
_authService.LogOut(() =>
@ -244,6 +255,8 @@ namespace Bit.App.Utilities.AccountManagement
AppResources.AccountAlreadyAdded, AppResources.Yes, AppResources.Cancel);
if (switchToAccount)
{
await _conditionedAwaiterManager.GetAwaiterForPrecondition(AwaiterPrecondition.EnvironmentUrlsInited);
await _stateService.SetActiveUserAsync(userId);
_messagingService.Send("switchedAccount");
}

View file

@ -0,0 +1,17 @@
using System;
using System.Threading.Tasks;
namespace Bit.Core.Abstractions
{
public enum AwaiterPrecondition
{
EnvironmentUrlsInited
}
public interface IConditionedAwaiterManager
{
Task GetAwaiterForPrecondition(AwaiterPrecondition awaiterPrecondition);
void SetAsCompleted(AwaiterPrecondition awaiterPrecondition);
void SetException(AwaiterPrecondition awaiterPrecondition, Exception ex);
}
}

View file

@ -0,0 +1,42 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Threading.Tasks;
using Bit.Core.Abstractions;
namespace Bit.Core.Services
{
public class ConditionedAwaiterManager : IConditionedAwaiterManager
{
private readonly ConcurrentDictionary<AwaiterPrecondition, TaskCompletionSource<bool>> _preconditionsTasks = new ConcurrentDictionary<AwaiterPrecondition, TaskCompletionSource<bool>>
{
[AwaiterPrecondition.EnvironmentUrlsInited] = new TaskCompletionSource<bool>()
};
public Task GetAwaiterForPrecondition(AwaiterPrecondition awaiterPrecondition)
{
if (_preconditionsTasks.TryGetValue(awaiterPrecondition, out var tcs))
{
return tcs.Task;
}
return Task.CompletedTask;
}
public void SetAsCompleted(AwaiterPrecondition awaiterPrecondition)
{
if (_preconditionsTasks.TryGetValue(awaiterPrecondition, out var tcs))
{
tcs.TrySetResult(true);
}
}
public void SetException(AwaiterPrecondition awaiterPrecondition, Exception ex)
{
if (_preconditionsTasks.TryGetValue(awaiterPrecondition, out var tcs))
{
tcs.TrySetException(ex);
}
}
}
}

View file

@ -3,6 +3,7 @@ using System.Threading.Tasks;
using Bit.Core.Abstractions;
using Bit.Core.Models.Data;
using Bit.Core.Models.Domain;
using Bit.Core.Utilities;
namespace Bit.Core.Services
{
@ -13,13 +14,16 @@ namespace Bit.Core.Services
private readonly IApiService _apiService;
private readonly IStateService _stateService;
private readonly IConditionedAwaiterManager _conditionedAwaiterManager;
public EnvironmentService(
IApiService apiService,
IStateService stateService)
IStateService stateService,
IConditionedAwaiterManager conditionedAwaiterManager)
{
_apiService = apiService;
_stateService = stateService;
_conditionedAwaiterManager = conditionedAwaiterManager;
}
public string BaseUrl { get; set; }
@ -51,6 +55,8 @@ namespace Bit.Core.Services
}
public async Task SetUrlsFromStorageAsync()
{
try
{
var urls = await _stateService.GetEnvironmentUrlsAsync();
if (urls == null)
@ -66,8 +72,11 @@ namespace Bit.Core.Services
{
BaseUrl = envUrls.Base = urls.Base;
_apiService.SetUrls(envUrls);
_conditionedAwaiterManager.SetAsCompleted(AwaiterPrecondition.EnvironmentUrlsInited);
return;
}
BaseUrl = urls.Base;
WebVaultUrl = urls.WebVault;
ApiUrl = envUrls.Api = urls.Api;
@ -76,6 +85,15 @@ namespace Bit.Core.Services
NotificationsUrl = urls.Notifications;
EventsUrl = envUrls.Events = urls.Events;
_apiService.SetUrls(envUrls);
_conditionedAwaiterManager.SetAsCompleted(AwaiterPrecondition.EnvironmentUrlsInited);
}
catch (System.Exception ex)
{
_conditionedAwaiterManager.SetException(AwaiterPrecondition.EnvironmentUrlsInited, ex);
throw;
}
}
public async Task<EnvironmentUrlData> SetUrlsAsync(EnvironmentUrlData urls)

View file

@ -33,6 +33,7 @@ namespace Bit.Core.Utilities
SearchService searchService = null;
var conditionedRunner = new ConditionedAwaiterManager();
var tokenService = new TokenService(stateService);
var apiService = new ApiService(tokenService, platformUtilsService, (extras) =>
{
@ -82,12 +83,13 @@ namespace Bit.Core.Utilities
keyConnectorService, passwordGenerationService);
var exportService = new ExportService(folderService, cipherService, cryptoService);
var auditService = new AuditService(cryptoFunctionService, apiService);
var environmentService = new EnvironmentService(apiService, stateService);
var environmentService = new EnvironmentService(apiService, stateService, conditionedRunner);
var eventService = new EventService(apiService, stateService, organizationService, cipherService);
var userVerificationService = new UserVerificationService(apiService, platformUtilsService, i18nService,
cryptoService);
var usernameGenerationService = new UsernameGenerationService(cryptoService, apiService, stateService);
Register<IConditionedAwaiterManager>(conditionedRunner);
Register<ITokenService>("tokenService", tokenService);
Register<IApiService>("apiService", apiService);
Register<IAppIdService>("appIdService", appIdService);

View file

@ -156,6 +156,20 @@ namespace Bit.iOS.Core.Utilities
ServiceContainer.Resolve<IAuthService>("authService").Init();
(ServiceContainer.
Resolve<IPlatformUtilsService>("platformUtilsService") as MobilePlatformUtilsService).Init();
var accountsManager = new AccountsManager(
ServiceContainer.Resolve<IBroadcasterService>("broadcasterService"),
ServiceContainer.Resolve<IVaultTimeoutService>("vaultTimeoutService"),
ServiceContainer.Resolve<IStorageService>("secureStorageService"),
ServiceContainer.Resolve<IStateService>("stateService"),
ServiceContainer.Resolve<IPlatformUtilsService>("platformUtilsService"),
ServiceContainer.Resolve<IAuthService>("authService"),
ServiceContainer.Resolve<ILogger>("logger"),
ServiceContainer.Resolve<IMessagingService>("messagingService"),
ServiceContainer.Resolve<IWatchDeviceService>(),
ServiceContainer.Resolve<IConditionedAwaiterManager>());
ServiceContainer.Register<IAccountsManager>("accountsManager", accountsManager);
// Note: This is not awaited
var bootstrapTask = BootstrapAsync(postBootstrapFunc);
}
@ -235,18 +249,6 @@ namespace Bit.iOS.Core.Utilities
ServiceContainer.Resolve<ICryptoService>("cryptoService"));
ServiceContainer.Register<IVerificationActionsFlowHelper>("verificationActionsFlowHelper", verificationActionsFlowHelper);
var accountsManager = new AccountsManager(
ServiceContainer.Resolve<IBroadcasterService>("broadcasterService"),
ServiceContainer.Resolve<IVaultTimeoutService>("vaultTimeoutService"),
ServiceContainer.Resolve<IStorageService>("secureStorageService"),
ServiceContainer.Resolve<IStateService>("stateService"),
ServiceContainer.Resolve<IPlatformUtilsService>("platformUtilsService"),
ServiceContainer.Resolve<IAuthService>("authService"),
ServiceContainer.Resolve<ILogger>("logger"),
ServiceContainer.Resolve<IMessagingService>("messagingService"),
ServiceContainer.Resolve<IWatchDeviceService>());
ServiceContainer.Register<IAccountsManager>("accountsManager", accountsManager);
if (postBootstrapFunc != null)
{
await postBootstrapFunc.Invoke();