diff --git a/App/MainWindow.cs b/App/MainWindow.cs index efc81a5..a96a158 100644 --- a/App/MainWindow.cs +++ b/App/MainWindow.cs @@ -1207,9 +1207,6 @@ private async Task LoadConfigurationAsync(string filePath) var config = await _configService.LoadAsync(filePath); - // Store metadata - _currentMetadata = config.Metadata; - // Connect to server and subscribe to nodes if (!string.IsNullOrEmpty(config.Server.EndpointUrl)) { @@ -1226,6 +1223,11 @@ private async Task LoadConfigurationAsync(string filePath) if (!pwDialog.Confirmed) { + // Nothing was torn down, but the load already switched the Ctrl+S + // target to this file; revert to untitled so a save cannot write + // the still-running session's state over it. + _configService.Reset(); + UpdateWindowTitle(); _logger.Info("Password prompt cancelled - skipping connection"); return; } @@ -1236,16 +1238,17 @@ private async Task LoadConfigurationAsync(string filePath) pwDialog.Password); } + // Tear down the current session first: stops any active recording and + // clears the views, so the UI cannot keep showing dead rows from the + // old server while (or after) the new connection is attempted. + await DisconnectAsync(); + var connected = await _connectionManager.ConnectAsync(config.Server.EndpointUrl, config.Settings.PublishingIntervalMs, credentials); if (connected) { _lastEndpoint = config.Server.EndpointUrl; - - // Only after successful connection, disconnect old and clear views - _addressSpaceView.Clear(); - _monitoredVariablesView.Clear(); - _nodeDetailsView.Clear(); + _currentMetadata = config.Metadata; _addressSpaceView.Initialize(_connectionManager.NodeBrowser); @@ -1271,24 +1274,25 @@ private async Task LoadConfigurationAsync(string filePath) } else { + // Revert to untitled: leaving the failed file as the Ctrl+S target + // would let a save overwrite it with the now-empty session state. + _configService.Reset(); + _currentMetadata = null; + UpdateWindowTitle(); + _logger.Error($"Failed to connect to {config.Server.EndpointUrl}"); - MessageBox.ErrorQuery("Connection Failed", - $"Could not connect to server:\n{config.Server.EndpointUrl}\n\nThe previous connection and data have been preserved.", + MessageBox.ErrorQuery("Connection Failed", + $"Could not connect to server:\n{config.Server.EndpointUrl}\n\nThe previous connection has been closed. Use Connect to reconnect.", "OK"); } } else { - // No endpoint URL, just clear views and apply settings - if (_connectionManager.IsConnected) - { - await _connectionManager.DisconnectAsync(); - } + // No endpoint URL: tear down any current session (stops recording, + // clears views) and just adopt the loaded settings. + await DisconnectAsync(); - _addressSpaceView.Clear(); - _monitoredVariablesView.Clear(); - _nodeDetailsView.Clear(); - + _currentMetadata = config.Metadata; _recentFiles.Add(filePath); UpdateWindowTitle(); _logger.Info("Configuration loaded (no server connection)"); diff --git a/OpcUa/ConnectionManager.cs b/OpcUa/ConnectionManager.cs index b96c877..b958538 100644 --- a/OpcUa/ConnectionManager.cs +++ b/OpcUa/ConnectionManager.cs @@ -123,7 +123,10 @@ public async Task ConnectAsync( string? securityMode = null, string? securityPolicy = null) { - Disconnect(); + // Async teardown: the synchronous Disconnect() blocks on the OPC UA close + // round-trip (up to the transport timeout against a dead server), which froze + // the UI thread when reconnecting over an existing or dead connection. + await DisconnectAsync(); _lastEndpoint = endpoint; _credentials = credentials ?? ConnectionCredentials.Anonymous; @@ -135,7 +138,17 @@ public async Task ConnectAsync( if (success) { - await InitializeSubscriptionAsync(publishingInterval); + if (!await InitializeSubscriptionAsync(publishingInterval)) + { + // Without a subscription the session is useless for monitoring; + // fail the connect rather than reporting Connected. + var msg = "Connected, but the server refused the monitoring subscription. Disconnecting."; + _logger.Error(msg); + ConnectionError?.Invoke(msg); + await DisconnectAsync(); + return false; + } + StateChanged?.Invoke(ConnectionState.Connected); } else @@ -300,11 +313,11 @@ public Task UnsubscribeAsync(uint clientHandle) return _client.WriteValueAsync(nodeId, value); } - private async Task InitializeSubscriptionAsync(int publishingInterval = 250) + private async Task InitializeSubscriptionAsync(int publishingInterval = 250) { _subscriptionManager = new SubscriptionManager(_client, _logger); _subscriptionManager.PublishingInterval = publishingInterval; - await _subscriptionManager.InitializeAsync(); + var initialized = await _subscriptionManager.InitializeAsync(); // Store handler references for proper unsubscription _valueChangedHandler = node => ValueChanged?.Invoke(node); @@ -314,6 +327,8 @@ private async Task InitializeSubscriptionAsync(int publishingInterval = 250) _subscriptionManager.ValueChanged += _valueChangedHandler; _subscriptionManager.VariableAdded += _variableAddedHandler; _subscriptionManager.VariableRemoved += _variableRemovedHandler; + + return initialized; } private void DisposeSubscription() diff --git a/OpcUa/OpcUaClientWrapper.cs b/OpcUa/OpcUaClientWrapper.cs index 50fcf5e..a68c12c 100644 --- a/OpcUa/OpcUaClientWrapper.cs +++ b/OpcUa/OpcUaClientWrapper.cs @@ -181,7 +181,7 @@ public async Task ConnectAsync( { try { - Disconnect(); + await DisconnectAsync(); _credentials = credentials ?? ConnectionCredentials.Anonymous; _securityMode = securityMode; @@ -231,14 +231,14 @@ public async Task ConnectAsync( : "Authentication rejected by server: " + sre.Message; _logger.Error(msg); ConnectionError?.Invoke(msg); - Disconnect(); + await DisconnectAsync(); return false; } catch (Exception ex) { _logger.Error($"Connection failed: {ex.Message}"); ConnectionError?.Invoke(ex.Message); - Disconnect(); + await DisconnectAsync(); return false; } } @@ -406,42 +406,26 @@ private async Task TryRecreateSessionAsync() var config = await GetApplicationConfigAsync(); - // Capture existing subscriptions before closing old session + var oldSession = _session; + + // Capture existing subscriptions before replacing the session SubscriptionCollection? subscriptionsToTransfer = null; - if (_session?.Subscriptions != null && _session.Subscriptions.Any()) + if (oldSession?.Subscriptions != null && oldSession.Subscriptions.Any()) { - subscriptionsToTransfer = new SubscriptionCollection(_session.Subscriptions); + subscriptionsToTransfer = new SubscriptionCollection(oldSession.Subscriptions); _logger.Info($"Captured {subscriptionsToTransfer.Count} subscription(s) for transfer"); } - // Clean up old session without deleting subscriptions on server - if (_session != null) - { - _session.KeepAlive -= Session_KeepAlive; - try - { - // Dispose without CloseAsync() - this intentionally skips sending CloseSession - // to the server, allowing server-side subscriptions to remain active for transfer. - // With DeleteSubscriptionsOnClose=false, we want the subscriptions to persist - // on the server so we can transfer them to the new session. - _session.Dispose(); - } - catch (Exception ex) - { - _logger.Warning($"Session cleanup error during reconnection: {ex.Message}"); - } - _session = null; - } - // Rediscover endpoint in case server configuration changed var selectedEndpoint = await DiscoverAndSelectEndpointAsync(config, _currentEndpoint, _securityMode, _securityPolicy); var endpointConfig = EndpointConfiguration.Create(config); var endpoint = new ConfiguredEndpoint(null, selectedEndpoint, endpointConfig); _lastConfiguredEndpoint = endpoint; - // Create new session + // Create the new session before touching the old one, so a failure here + // leaves the existing state unchanged for the next retry attempt. #pragma warning disable CS0618 - _session = await Opc.Ua.Client.Session.Create( + var newSession = await Opc.Ua.Client.Session.Create( config, endpoint, false, @@ -452,18 +436,46 @@ private async Task TryRecreateSessionAsync() ); #pragma warning restore CS0618 - _session.DeleteSubscriptionsOnClose = false; - _session.TransferSubscriptionsOnReconnect = true; - _session.KeepAlive += Session_KeepAlive; + newSession.DeleteSubscriptionsOnClose = false; + newSession.TransferSubscriptionsOnReconnect = true; + newSession.KeepAlive += Session_KeepAlive; + _session = newSession; - // Transfer subscriptions to new session - if (subscriptionsToTransfer != null && subscriptionsToTransfer.Count > 0) + // Transfer subscriptions while the old session is still alive. The client-side + // half of TransferSubscriptionsAsync detaches each subscription from its previous + // session; if that session is already disposed this throws, the SDK swallows it + // and reports failure - after the server-side transfer already succeeded - leaving + // orphaned subscriptions on the server and forcing a duplicate recreate. + try + { + if (subscriptionsToTransfer != null && subscriptionsToTransfer.Count > 0) + { + var transferred = await TransferSubscriptionsAsync(subscriptionsToTransfer); + _logger.Info($"Transferred {transferred} of {subscriptionsToTransfer.Count} subscription(s)"); + } + } + finally { - var transferred = await TransferSubscriptionsAsync(subscriptionsToTransfer); - _logger.Info($"Transferred {transferred} of {subscriptionsToTransfer.Count} subscription(s)"); + // Retire the old session even if the transfer throws - once _session + // points at the new session the old one would otherwise leak. Dispose + // without CloseAsync() - sending CloseSession would delete any + // server-side subscriptions that were not transferred, and the + // transport is typically already dead on this path. + if (oldSession != null) + { + oldSession.KeepAlive -= Session_KeepAlive; + try + { + oldSession.Dispose(); + } + catch (Exception ex) + { + _logger.Warning($"Session cleanup error during reconnection: {ex.Message}"); + } + } } - return _session.Connected; + return newSession.Connected; } catch (Exception ex) {