Code Review s Architektem, Round #2 - thread-safe sdílení spojení pro více klientů rubrika: Programování: .Net

2 Jaroslav Urban
položil/-a 29.1.2014

Zdravim,
mám tu pro Vás další scénář k diskuzi. Tak trochu mě momentálně nesedí řešení, které používáme.

Scenář:

  • jde o komponentu která zajištujě spojení pro více klientských komponent k další komponentě přes WCF. Požadavkem je mít jen jedno aktivní spojení. Tedy pro prvního klienta se otevře spojení voláním metodou Connect, a pro každého dalšího se použije již existující spojení. Obráceně pro odpojení se zavolá Disconnect až pro posledního klienta. Jde o to aby se volal Connect a Disconnect jen jednou, na začátku a na konci. V příkladě jde o instanci McaClient, viz kód níže.
  • při odpojení posledního klienta dojde ke kompletnímu dispose komponenty zajišťující spojení
  • thread safe nebo navrženo tak aby se thread safe nemuselo řešit.
  • při zachycené chybě dojde k odpojení všech klientů
  • C# 4.0 only
  • podle mě tohle není až tak specifický scénář, takže ho již někde, někdo řešil

Momentální řešení:

  • použití HashSet kolekce pro evidenci klientů
  • použití locku pro přístup ke kolekci klientů
  • použití locku pro přístup k proměnné vracející stav připojení (bool IsConnectedToService)

A co mě vlastně vadí:

  • locky, šlo by to bez nich? minimálně bez toho se stavem připojení
  • volání metod injektovaných component v locku.

Doufám, že z toho něco přečtěte, bez znalosti všech detailů to nemusí být úplně jasné. Podstatou otázky ale je jestli existuje bezpečné řešení bez použití locků, případně jestli některé nelze eliminovat jiným přístupem k danému problému. Děkuji za vaše případné příspěvky. JU

public class McaController : IMcaController, IDisposable
{
    private readonly object m_ClientsControllingLock = new object();
    private readonly HashSet<object> m_ConnectedClients;
    private readonly IMcaClientFactory m_McaClientFactory;
    private bool m_Disposed;
    private IMcaClient m_McaClient; // component handling connection
 
    /// <summary>
    /// Ctor.
    /// </summary>
    /// <param name="mcaClientFactory">
    /// Factory for clients of Mca Service.
    /// </param>
    public McaController(IMcaClientFactory mcaClientFactory)
    {
        if (mcaClientFactory == null) throw new ArgumentNullException("mcaClientFactory");
 
        m_McaClientFactory = mcaClientFactory;
        m_ConnectedClients = new HashSet<object>();
    }
 
    /// <summary>
    /// Finalizes an instance of the <see cref="McaController"/> class.
    /// </summary>
    ~McaController()
    {
        Dispose(false);
    }
 
    /// <summary>
    /// Occurs when confirmation of critical command is expired.
    /// No accept or decline action.
    /// </summary>
    public event Action<IMaqbusCommand> ConfirmationExpired;
 
    /// <summary>
    /// Occurs when confirmation of critical command is requested.
    /// </summary>
    public event Action<IMaqbusCommand> ConfirmationRequested;
 
    /// <summary>
    /// Occurs when [error handler].
    /// </summary>
    public event Action<Exception> ErrorHandler;
 
    /// <summary>
    /// When status updated.
    /// </summary>
    public event Action<IStatusDescription> StatusUpdated;
 
    private bool IsConnectedToService
    {
        get
        {
            return m_McaClient != null;
        }
    }
 
    /// <summary>
    /// Confirm the command.
    /// </summary>
    /// <param name="command">The command to be confirmed.</param>
    public void AcceptConfirmation(IMaqbusCommand command)
    {
        LogMediator.LogInfo(() => AcceptConfirmation(command), "Call AcceptConfirmation for command: {0} ...", command);
 
        lock (m_ClientsControllingLock)
        {
            CheckPreconditions(command);
            m_McaClient.AcceptConfirmation(command);
        }
    }
 
    /// <summary>
    /// Connects to MCA and registers given client to collection of connected clients.
    /// </summary>
    /// <param name="client">The client.</param>
    public void Connect(object client)
    {
        if (client == null) throw new ArgumentNullException("client");
 
        lock (m_ClientsControllingLock)
        {
            AddClientToConnected(client);
            InitializeMcaClient();
            CallConnectOnService(); // JU: this might take some time
        }
    }
 
    /// <summary>
    /// Declines the command's confirmation.
    /// </summary>
    /// <param name="command">The command not to be confirmed.</param>
    public void DeclineConfirmation(IMaqbusCommand command)
    {
        LogMediator.LogInfo(() => DeclineConfirmation(command), "Call DeclineConfirmation for command: {0} ...", command);
 
        lock (m_ClientsControllingLock)
        {
            CheckPreconditions(command);
            m_McaClient.DeclineConfirmation(command);
        }
    }
 
    /// <summary>
    /// Removes client from collection of connected clients. If there are no aother connected clients, disconnects from MCA.
    /// </summary>
    /// <param name="client">The client.</param>
    public void Disconnect(object client)
    {
        if (client == null) throw new ArgumentNullException("client");
        IMcaClient mcaClientToBeReleased = null;
 
        lock (m_ClientsControllingLock)
        {
            RemoveClientFromConnected(client);
 
            if (ShouldDisconnect())
            {
                mcaClientToBeReleased = m_McaClient;
                m_McaClient = null;
            }
        }
 
        ReleaseMcaClient(mcaClientToBeReleased);
    }
 
    /// <summary>
    /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
    /// </summary>
    /// <filterpriority>2</filterpriority>
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
 
    /// <summary>
    /// Runs a command.
    /// </summary>
    /// <param name="command">Command.</param>
    public void Execute(IMaqbusCommand command)
    {
        LogMediator.LogInfo(() => Execute(command), "{0} is executing on McaController ...", command);
 
        lock (m_ClientsControllingLock)
        {
            CheckPreconditions(command);
            m_McaClient.Execute(command);
        }
    }
 
    private void AddClientToConnected(object client)
    {
        Debug.WriteLine("AddClientToConnected: '{0}'", client);
 
        if (!m_ConnectedClients.Contains(client))
        {
            m_ConnectedClients.Add(client);
        }
    }
 
    private void CallConnectOnService()
    {
        LogMediator.LogInfo(() => CallConnectOnService(), "Call Connect() on MCA service.");
        m_McaClient.Connect();
    }
 
    private void DisconnectAllClients()
    {
        IMcaClient mcaClientToBeReleased;
 
        lock (m_ClientsControllingLock)
        {
            m_ConnectedClients.Clear();
            mcaClientToBeReleased = m_McaClient;
            m_McaClient = null;
        }
 
        ReleaseMcaClient(mcaClientToBeReleased);
    }
 
    private void ReleaseMcaClient(IMcaClient mcaClient)
    {
        if (mcaClient != null)
        {
            mcaClient.StatusUpdated -= OnStatusUpdated;
            mcaClient.ConfirmationRequested -= OnConfirmationRequested;
            mcaClient.ConfirmationExpired -= OnConfirmationExpired;
            mcaClient.ErrorHandler -= OnErrorHandler;
            m_McaClientFactory.Release(mcaClient);
        }
    }
 
    private void Dispose(bool disposing)
    {
        if (!m_Disposed && disposing)
        {
            ReleaseMcaClient(m_McaClient);
            m_McaClient = null;
        }
 
        m_Disposed = true;
    }
 
    private void CheckPreconditions(IMaqbusCommand command)
    {
        if (command == null) throw new ArgumentNullException("command");
        if (!IsConnectedToService) throw new InvalidOperationException("Don't call Execute() when not connected.");
    }
 
    private void OnConfirmationExpired(IMaqbusCommand command)
    {
        if (IsConnectedToService)
        {
            RaiseConfirmationExpired(command);
        }
    }
 
    private void OnConfirmationRequested(IMaqbusCommand command)
    {
        if (IsConnectedToService)
        {
            RaiseConfirmationRequested(command);
        }
    }
 
    private void OnErrorHandler(Exception exception)
    {
        if (exception is McaMinorException)
        {
            LogMediator.LogWarn(() => OnErrorHandler(exception), exception.Message);
            return;
        }
 
        if (!IsExceptionCriticalOrAccessDenied(exception))
        {
            DisconnectAllClients();
        }
 
        LogMediator.LogError(() => OnErrorHandler(exception), exception.ToString());
        RaiseErrorHandler(exception);
    }
 
    private void OnStatusUpdated(IStatusDescription statusDescription)
    {
        RaiseStatusUpdated(statusDescription);
    }
 
    private void RaiseConfirmationExpired(IMaqbusCommand command)
    {
        LogMediator.LogWarn(() => RaiseConfirmationExpired(command), "Raising ConfirmationExpired for command: {0} ...", command);
 
        var handler = ConfirmationExpired;
 
        if (handler != null) handler(command);
    }
 
    private void RaiseConfirmationRequested(IMaqbusCommand command)
    {
        LogMediator.LogInfo(() => RaiseConfirmationRequested(command), "Raising ConfirmationRequest for command: {0} ...", command);
 
        var handler = ConfirmationRequested;
 
        if (handler != null) handler(command);
    }
 
    private void RaiseErrorHandler(Exception exception)
    {
        ErrorHandler.InvokeIfNotNull(exception);
    }
 
    private void RaiseStatusUpdated(IStatusDescription statusDescription)
    {
        var handler = StatusUpdated;
 
        if (handler != null) handler(statusDescription);
    }
 
    private void RemoveClientFromConnected(object client)
    {
        Debug.WriteLine("RemoveClientFromConnected: '{0}'", client);
 
        m_ConnectedClients.Remove(client);
    }
 
    private bool ShouldDisconnect()
    {
        return IsConnectedToService && !m_ConnectedClients.Any();
    }
 
    private bool IsExceptionCriticalOrAccessDenied(Exception exception)
    {
        return exception is McaCriticalException || exception is McaAccesssDeniedException;
    }
 
    private void InitializeMcaClient()
    {
        if (!IsConnectedToService)
        {
            LogMediator.LogInfo(() => InitializeMcaClient(), "MCA client is creating ...");
            m_McaClient = m_McaClientFactory.Create();
            m_McaClient.ErrorHandler += OnErrorHandler;
            m_McaClient.StatusUpdated += OnStatusUpdated;
            m_McaClient.ConfirmationRequested += OnConfirmationRequested;
            m_McaClient.ConfirmationExpired += OnConfirmationExpired;
        }
    }
}
odkaz
9 joinmax
odpověděl/-a 29.1.2014

To vypadá na typický pooling zdrojů - ten by měl řešit framework/platforma/knihovna, ne aplikace. Tzn. máš si jen nastavit velikost poolu a další pravidla a nic víc už sám neřešit.

Nebo to špatně chápu a ty píšeš právě tu obecnou knihovnu/framework?

Komentáře

Pro zobrazení všech 3 odpovědí se prosím přihlaste:

Rychlé přihlášení přes sociální sítě:

Nebo se přihlaste jménem a heslem:

Zadejte prosím svou e-mailovou adresu.
Zadejte své heslo.