Skip to content

Fix connection lifecycle: transfer-before-dispose reconnect, async connect teardown, truthful config load #167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
BrettKinny merged 2 commits into from
Jun 11, 2026
Merged

Fix connection lifecycle: transfer-before-dispose reconnect, async connect teardown, truthful config load #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a78740c
Fix connection lifecycle: transfer-before-dispose reconnect, async co…
claude Jun 10, 2026
ee7302b
Dispose old session in finally during session recreation
claude Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 23 additions & 19 deletions App/MainWindow.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand All @@ -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;
}
Expand All @@ -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);

Expand All @@ -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)");
Expand Down
23 changes: 19 additions & 4 deletions OpcUa/ConnectionManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,10 @@ public async Task<bool> 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;
Expand All @@ -135,7 +138,17 @@ public async Task<bool> 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
Expand Down Expand Up @@ -300,11 +313,11 @@ public Task<bool> UnsubscribeAsync(uint clientHandle)
return _client.WriteValueAsync(nodeId, value);
}

private async Task InitializeSubscriptionAsync(int publishingInterval = 250)
private async Task<bool> 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);
Expand All @@ -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()
Expand Down
82 changes: 47 additions & 35 deletions OpcUa/OpcUaClientWrapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public async Task<bool> ConnectAsync(
{
try
{
Disconnect();
await DisconnectAsync();

_credentials = credentials ?? ConnectionCredentials.Anonymous;
_securityMode = securityMode;
Expand Down Expand Up @@ -231,14 +231,14 @@ public async Task<bool> 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;
}
}
Expand Down Expand Up @@ -406,42 +406,26 @@ private async Task<bool> 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,
Expand All @@ -452,18 +436,46 @@ private async Task<bool> 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)
{
Expand Down
Loading