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?
Pro zobrazení všech 3 odpovědí se prosím přihlaste:
Nebo se přihlaste jménem a heslem:
Komentáře