1/2
Hi Rondel,
I will answer your questions about singletons and thread safety:
"The vast majority of our service registrations are singletons including the FileSystemProvider though I'm not sure that should be a problem. Is the FileSystemProvider designed to be thread safe or should I be creating and disposing of instances for each thread."
From the description of the system design you have provided, you are using the instances with the following lifecycles.
1) Singleton instance of the Azure SDK class(es).
2) Singleton instance of the AzureFileSystemProvider (instance of your class) decorated by the "child file system" (user session-scoped instance).
3) Singleton/Session scoped instance of the user.
Dependencies and its lifecycle (simplified):
AzureSdkClass{Singleton} <- AzureFileSystemProvider{Singleton} <- ChildFileSystem(user session-scoped ) <-FileServerUser {Singleton or user-session scoped}.
This design exhibits the following liabilities.
1) All requests are served by only one instance of the AzureFileSystemProvider. Every FileSystemProvider returned from the CreateChildFileSystem method is a decorator that only appends the path to the request and then forwards the request to the underlying provider. In our case to the instance of the AzureFileSystemProvider.
2) When this singleton instance of the AzureFileSystemProvider becomes corrupted or when some virtual file system operation is delayed/got stuck then all requests for all users might be affected.
So:
Rule 1) DON'T use a single instance of the file system provider for all requests on the SFTP server. Every rule has exceptions, but really don't try to share instances until it is necessary because the underlying virtual file system storage has special requirements.
Rule 2) PREFER user session-scoped file system provider. When one instance of the file system starts to behave badly, only one user session will be affected. All other user sessions will happily live in their own safe and by the fatal incident in the foreign session unaffected mini-world.
Concretely for your scenario.
Handle authentication events.
//Handle Authentication event to authenticate users
_sftpServer.Authentication += (sender, args) => SftpServerAuthentication(sender, args);
//Handle disconnected event to Dispose instance of a AzureFileServerUser.
_sftpServer.Disconnected += SftpServerDisconnected;
SftpServerAuthentication event handler.
private static void SftpServerAuthentication(object sender, AuthenticationEventArgs e)
{
var currentUserName = e.UserName;
//Load a user from database.
var user = tryAuthenticateUser(currentUserName);
//When a user is not in our database, deny access.
if (user == null)
{
e.Reject();
return;
}
//Verify user password
if (!_passwordHashService.VerifyPassword(user, e.Password))
{
//When user sends invalid password, deny access.
e.Reject();
return;
}
//Create "user session scoped" instance of the AzureFileSystemProvider.
var azureFileSystemProvider = new AzureFileSystemProvider(...);
//Create instance of our AzureFileServerUser with user specific file system.
var azureFileServerUser = new AzureFileServerUser(currentUserName, azureFileSystemProvider);
//Authentication succeeded. Allow user logon.
e.Accept(azureFileServerUser);
}
SftpServerDisconnected event handler.
/// <summary>
/// Handler for the <see cref="Server.Disconnected"/> event.
/// </summary>
private static void SftpServerDisconnected(object sender, DisconnectedEventArgs e)
{
//Calling the Dispose method on the instance of the AzureFileServerUser
//initiates disposing of the chain AzureFileServerUser->AzureFileSystemProvider
//In other words, all "user session scoped" services are disposed.
if (e.User is AzureFileServerUser azureFileServerUser)
{
azureFileServerUser.Dispose();
}
}
And AzureFileServerUser might look like this.
/// <summary>
/// Class represents SFTP user derived from the Rebex <see cref="FileServerUser"/>.
/// Instances of this class are created after successful authentication.
/// </summary>
public class AzureFileServerUser : FileServerUser, IDisposable
{
//"user session scoped" AzureFileSystemProvider.
private readonly AzureFileSystemProvider _provider;
/// <summary>
/// Initialized a new instance of the <see cref="AzureFileServerUser"/>.
/// </summary>
/// <param name="userName">User name</param>
/// <param name="provider">"user session scoped" instance of the AzureFileSystemProvider</param>
public AzureFileServerUser(string userName, AzureFileSystemProvider provider, string virtualRoot) : base(userName, String.Empty, provider, string virtualRoot)
{
_provider = provider ?? throw new ArgumentNullException(nameof(provider));
}
/// <summary>Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.</summary>
public void Dispose()
{
Dispose(true);
}
/// <summary>
/// Dispose "user session scoped" services.
/// </summary>
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
//Dispose AzureFileSystemProvider.
_provider.Dispose();
}
}
}
}
I don't want to sound like the stuck record but let me reiterate.
FileSystemProvider with singleton lifecycle represents a single point of failure. And please be vigilant and in your case double-check guarantees in Azure SDK because you are still using Azure SDK classes with the singleton life cycle.
commented
by
Lukas Pokorny
(147k points)