Keep subtag acks and alarm reads safe on the managed ack path
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (pull_request) Successful in 2m0s
ci / nightly-windev (pull_request) Has been skipped
ci / java (pull_request) Successful in 2m16s
ci / java (push) Successful in 2m19s
ci / portable (push) Failing after 2m31s
ci / portable (pull_request) Failing after 2m36s
ci / windows-x86 (push) Successful in 3m11s
ci / nightly-windev (push) Has been skipped
ci / windows-x86 (pull_request) Successful in 2m0s
ci / nightly-windev (pull_request) Has been skipped
ci / java (pull_request) Successful in 2m16s
ci / java (push) Successful in 2m19s
ci / portable (push) Failing after 2m31s
ci / portable (pull_request) Failing after 2m36s
ci / windows-x86 (push) Successful in 3m11s
Review fixes for the opt-in managed GUID ack: - Subtag-mode GUID acks (forced subtag, or failover on the standby) stay on the consumer; their GUIDs are synthetic and only the subtag consumer can resolve them. Forced-subtag sessions no longer build a managed client. - A failed managed Initialize/Register/Subscribe drops back to the wnwrap ack path instead of failing the alarm subscription. - Drop stale crefs to a nonexistent Unverified member in WnalNativeAckConsumer. - Unit tests for routing, fallback, and disposal; document MXGATEWAY_USE_MANAGED_ALARM_ACK. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -793,6 +793,28 @@ in the codebase for the forward-compat shape, but the gateway-side
|
||||
`AcknowledgeAlarmByName` when the public RPC supplies a recognizable
|
||||
`Provider!Group.Tag` reference.
|
||||
|
||||
**Opt-in managed GUID ack.** Setting `MXGATEWAY_USE_MANAGED_ALARM_ACK=1`
|
||||
in the gateway's environment (workers inherit it) routes GUID-keyed acks
|
||||
through `aaAlarmManagedClient.AlarmClient.AlarmAckByGUID` instead of the
|
||||
wnwrap method. That managed method is implemented on this build and
|
||||
carries operator domain and full name natively. The worker reaches it by
|
||||
reflection (`AaManagedAlarmAckConsumer`) because the members are
|
||||
non-public, and runs its own Initialize → Register → Subscribe lifecycle
|
||||
against the alarm subscription expression. The path is deliberately
|
||||
conservative:
|
||||
|
||||
- If the DLL cannot load or the lifecycle returns a non-zero status, the
|
||||
worker drops back to the wnwrap ack path; alarm reads never fail
|
||||
because of it.
|
||||
- Subtag-mode acks (forced subtag, or failover currently on the standby)
|
||||
stay on the subtag consumer: their GUIDs are synthetic and only it can
|
||||
resolve them.
|
||||
- By-name acks stay on wnwrap — `AlarmClient` has no by-name ack.
|
||||
|
||||
A zero return still means *accepted*, not *applied*; a secured
|
||||
(`MxSecurityOperate`) alarm needs an authenticated operator. The skip-gated
|
||||
`AlarmManagedGuidAckLiveSmokeTests` asserts the ack actually lands.
|
||||
|
||||
### 5. STA / threading — resolved
|
||||
|
||||
The wnwrap COM is `ThreadingModel=Apartment`. The consumer's
|
||||
|
||||
@@ -446,6 +446,13 @@ behavior.
|
||||
The alarm monitor is independent of client sessions: `AcknowledgeAlarm` and
|
||||
`StreamAlarms` are session-less RPCs served by the monitor.
|
||||
|
||||
GUID-keyed acknowledges can opt into AVEVA's managed alarm client by setting
|
||||
the environment variable `MXGATEWAY_USE_MANAGED_ALARM_ACK=1` on the gateway
|
||||
process (workers inherit it; under NSSM, add it to the service environment).
|
||||
It is off by default and has no `MxGateway:*` option because it is still
|
||||
awaiting live confirmation on the rig. See
|
||||
[AlarmClientDiscovery](./AlarmClientDiscovery.md) ("Opt-in managed GUID ack").
|
||||
|
||||
### Alarm fallback options
|
||||
|
||||
The `Fallback` sub-section controls how the alarm feed selects between the
|
||||
|
||||
@@ -203,6 +203,100 @@ public sealed class AlarmDispatcherTests
|
||||
Assert.Equal("Alice Smith", consumer.LastAckOperatorFullName);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that a managed ack client takes GUID acks with full operator identity.</summary>
|
||||
[Fact]
|
||||
public void Acknowledge_WithManagedAckClient_RoutesGuidAckToManagedClient()
|
||||
{
|
||||
FakeAlarmConsumer consumer = new FakeAlarmConsumer { AcknowledgeReturn = -1 };
|
||||
FakeManagedAckClient managed = new FakeManagedAckClient { AcknowledgeReturn = 0 };
|
||||
using AlarmDispatcher dispatcher = new AlarmDispatcher(
|
||||
consumer,
|
||||
new MxAccessAlarmEventSink(new MxAccessEventQueue(), new MxAccessEventMapper()),
|
||||
SessionId,
|
||||
managed);
|
||||
dispatcher.Subscribe(@"\\HOST\Galaxy!Area1");
|
||||
|
||||
Guid guid = Guid.NewGuid();
|
||||
int rc = dispatcher.Acknowledge(guid, "Acked", "alice", "WS01", "CORP", "Alice Smith");
|
||||
|
||||
Assert.Equal(0, rc);
|
||||
Assert.Equal(@"\\HOST\Galaxy!Area1", managed.LastSubscription);
|
||||
Assert.Equal(guid, managed.LastAckGuid);
|
||||
Assert.Equal("CORP", managed.LastAckOperatorDomain);
|
||||
Assert.Equal("Alice Smith", managed.LastAckOperatorFullName);
|
||||
Assert.Equal(Guid.Empty, consumer.LastAckGuid);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that a failed managed-client lifecycle neither fails the alarm
|
||||
/// subscription nor leaves acks routed to the broken client.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Subscribe_WhenManagedAckClientFails_FallsBackToConsumerAck()
|
||||
{
|
||||
FakeAlarmConsumer consumer = new FakeAlarmConsumer { AcknowledgeReturn = 0 };
|
||||
FakeManagedAckClient managed = new FakeManagedAckClient { ThrowOnSubscribe = true };
|
||||
using AlarmDispatcher dispatcher = new AlarmDispatcher(
|
||||
consumer,
|
||||
new MxAccessAlarmEventSink(new MxAccessEventQueue(), new MxAccessEventMapper()),
|
||||
SessionId,
|
||||
managed);
|
||||
|
||||
dispatcher.Subscribe(@"\\HOST\Galaxy!Area1");
|
||||
Guid guid = Guid.NewGuid();
|
||||
int rc = dispatcher.Acknowledge(guid, "Acked", "alice", "WS01", "CORP", "Alice Smith");
|
||||
|
||||
Assert.Equal(@"\\HOST\Galaxy!Area1", consumer.LastSubscription);
|
||||
Assert.True(managed.Disposed);
|
||||
Assert.Equal(0, rc);
|
||||
Assert.Equal(guid, consumer.LastAckGuid);
|
||||
Assert.Equal(Guid.Empty, managed.LastAckGuid);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Verifies that while the failover consumer serves the subtag standby,
|
||||
/// GUID acks (synthetic GUIDs) stay on the consumer, not the managed client.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Acknowledge_WhenFailoverInSubtagMode_KeepsGuidAckOnConsumer()
|
||||
{
|
||||
FakeAlarmConsumer primary = new FakeAlarmConsumer { ThrowOnSubscribe = true };
|
||||
FakeAlarmConsumer standby = new FakeAlarmConsumer { AcknowledgeReturn = 0 };
|
||||
FailoverAlarmConsumer failover = new FailoverAlarmConsumer(
|
||||
primary, standby, new FailoverSettings(threshold: 1, probeIntervalSeconds: 0, stableProbes: 3));
|
||||
FakeManagedAckClient managed = new FakeManagedAckClient { AcknowledgeReturn = -1 };
|
||||
using AlarmDispatcher dispatcher = new AlarmDispatcher(
|
||||
failover,
|
||||
new MxAccessAlarmEventSink(new MxAccessEventQueue(), new MxAccessEventMapper()),
|
||||
SessionId,
|
||||
managed);
|
||||
dispatcher.Subscribe(@"\\HOST\Galaxy!Area1");
|
||||
Assert.Equal(AlarmProviderMode.Subtag, failover.Mode);
|
||||
|
||||
Guid guid = Guid.NewGuid();
|
||||
int rc = dispatcher.Acknowledge(guid, "Acked", "alice", "WS01", "CORP", "Alice Smith");
|
||||
|
||||
Assert.Equal(0, rc);
|
||||
Assert.Equal(guid, standby.LastAckGuid);
|
||||
Assert.Equal(Guid.Empty, managed.LastAckGuid);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that disposing the dispatcher disposes the managed ack client.</summary>
|
||||
[Fact]
|
||||
public void Dispose_WithManagedAckClient_DisposesManagedClient()
|
||||
{
|
||||
FakeManagedAckClient managed = new FakeManagedAckClient();
|
||||
AlarmDispatcher dispatcher = new AlarmDispatcher(
|
||||
new FakeAlarmConsumer(),
|
||||
new MxAccessAlarmEventSink(new MxAccessEventQueue(), new MxAccessEventMapper()),
|
||||
SessionId,
|
||||
managed);
|
||||
|
||||
dispatcher.Dispose();
|
||||
|
||||
Assert.True(managed.Disposed);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that acknowledge-by-name calls are forwarded to the consumer.</summary>
|
||||
[Fact]
|
||||
public void AcknowledgeByName_WhenInvoked_ForwardsToConsumerWithFullTuple()
|
||||
@@ -470,9 +564,13 @@ public sealed class AlarmDispatcherTests
|
||||
AlarmTransitionEmitted?.Invoke(this, transition);
|
||||
}
|
||||
|
||||
/// <summary>Gets or sets a value indicating whether <see cref="Subscribe"/> throws.</summary>
|
||||
public bool ThrowOnSubscribe { get; set; }
|
||||
|
||||
/// <inheritdoc />
|
||||
public void Subscribe(string subscription)
|
||||
{
|
||||
if (ThrowOnSubscribe) throw new InvalidOperationException("subscribe failed");
|
||||
LastSubscription = subscription;
|
||||
}
|
||||
|
||||
@@ -533,4 +631,50 @@ public sealed class AlarmDispatcherTests
|
||||
Disposed = true;
|
||||
}
|
||||
}
|
||||
|
||||
private sealed class FakeManagedAckClient : IManagedAlarmAckClient
|
||||
{
|
||||
/// <summary>Gets the last subscription expression.</summary>
|
||||
public string? LastSubscription { get; private set; }
|
||||
/// <summary>Gets the GUID from the last acknowledge call.</summary>
|
||||
public Guid LastAckGuid { get; private set; }
|
||||
/// <summary>Gets the operator domain from the last acknowledge call.</summary>
|
||||
public string? LastAckOperatorDomain { get; private set; }
|
||||
/// <summary>Gets the operator full name from the last acknowledge call.</summary>
|
||||
public string? LastAckOperatorFullName { get; private set; }
|
||||
/// <summary>Gets or sets the return code for acknowledge operations.</summary>
|
||||
public int AcknowledgeReturn { get; set; }
|
||||
/// <summary>Gets or sets a value indicating whether <see cref="Subscribe"/> throws.</summary>
|
||||
public bool ThrowOnSubscribe { get; set; }
|
||||
/// <summary>Gets a value indicating whether this instance has been disposed.</summary>
|
||||
public bool Disposed { get; private set; }
|
||||
|
||||
/// <inheritdoc />
|
||||
public void Subscribe(string subscription)
|
||||
{
|
||||
if (ThrowOnSubscribe) throw new InvalidOperationException("RegisterConsumer returned 5.");
|
||||
LastSubscription = subscription;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public int AcknowledgeByGuid(
|
||||
Guid alarmGuid,
|
||||
string comment,
|
||||
string operatorName,
|
||||
string operatorNode,
|
||||
string operatorDomain,
|
||||
string operatorFullName)
|
||||
{
|
||||
LastAckGuid = alarmGuid;
|
||||
LastAckOperatorDomain = operatorDomain;
|
||||
LastAckOperatorFullName = operatorFullName;
|
||||
return AcknowledgeReturn;
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public void Dispose()
|
||||
{
|
||||
Disposed = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -182,7 +182,11 @@ public sealed class AlarmCommandHandler : IAlarmCommandHandler
|
||||
}
|
||||
|
||||
// Optional managed ack client (opt-in via env); null → wnwrap ack.
|
||||
IManagedAlarmAckClient? managedAck = managedAckFactory?.Invoke();
|
||||
// A forced-subtag session never has alarmmgr GUIDs to ack, so it
|
||||
// skips the extra managed consumer entirely.
|
||||
IManagedAlarmAckClient? managedAck = command.ForcedMode == AlarmProviderMode.Subtag
|
||||
? null
|
||||
: managedAckFactory?.Invoke();
|
||||
MxAccessAlarmEventSink sink = new MxAccessAlarmEventSink(eventQueue, mapper);
|
||||
dispatcher = new AlarmDispatcher(consumer, sink, subscribeSessionId, managedAck);
|
||||
try
|
||||
|
||||
@@ -36,7 +36,7 @@ public sealed class AlarmDispatcher : IDisposable
|
||||
private readonly MxAccessAlarmEventSink sink;
|
||||
private readonly string sessionId;
|
||||
private readonly EventHandler<MxAlarmTransitionEvent> handler;
|
||||
private readonly IManagedAlarmAckClient? managedAck;
|
||||
private IManagedAlarmAckClient? managedAck;
|
||||
private bool disposed;
|
||||
|
||||
/// <summary>Initializes a new alarm dispatcher for the given consumer, sink, and session ID.</summary>
|
||||
@@ -46,7 +46,9 @@ public sealed class AlarmDispatcher : IDisposable
|
||||
/// <param name="managedAck">
|
||||
/// Optional managed ack client. When non-null, GUID-keyed acknowledges
|
||||
/// route to it (AVEVA's modern <c>AlarmClient.AlarmAckByGUID</c>) instead
|
||||
/// of the wnwrap consumer; null keeps the existing wnwrap ack path.
|
||||
/// of the wnwrap consumer while the alarmmgr provider is active; null
|
||||
/// keeps the existing wnwrap ack path. Subtag-mode acks always stay on
|
||||
/// the consumer because their GUIDs are synthetic.
|
||||
/// </param>
|
||||
public AlarmDispatcher(
|
||||
IMxAccessAlarmConsumer consumer,
|
||||
@@ -80,10 +82,31 @@ public sealed class AlarmDispatcher : IDisposable
|
||||
consumer.Subscribe(subscription);
|
||||
// The managed ack client maintains its own registered/subscribed
|
||||
// consumer; run its lifecycle against the same expression so a later
|
||||
// AlarmAckByGUID has a live consumer to act through.
|
||||
managedAck?.Subscribe(subscription);
|
||||
// AlarmAckByGUID has a live consumer to act through. It is an opt-in
|
||||
// enhancement, so a failed lifecycle drops back to the wnwrap ack path
|
||||
// rather than failing the alarm subscription.
|
||||
if (managedAck is not null)
|
||||
{
|
||||
try
|
||||
{
|
||||
managedAck.Subscribe(subscription);
|
||||
}
|
||||
catch
|
||||
{
|
||||
try { managedAck.Dispose(); } catch { /* swallow */ }
|
||||
managedAck = null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// True when the consumer is currently serving alarms from the subtag
|
||||
/// fallback, whose alarm GUIDs are synthetic and unknown to alarmmgr.
|
||||
/// </summary>
|
||||
private bool IsSubtagActive() =>
|
||||
consumer is SubtagAlarmConsumer ||
|
||||
(consumer is FailoverAlarmConsumer failover && failover.Mode == AlarmProviderMode.Subtag);
|
||||
|
||||
/// <summary>
|
||||
/// Forward an <c>AcknowledgeAlarm</c> request to the underlying
|
||||
/// consumer's <c>AlarmAckByGUID</c>. Returns the AVEVA-native
|
||||
@@ -105,11 +128,13 @@ public sealed class AlarmDispatcher : IDisposable
|
||||
string ackOperatorFullName)
|
||||
{
|
||||
if (disposed) throw new ObjectDisposedException(nameof(AlarmDispatcher));
|
||||
if (managedAck is not null)
|
||||
if (managedAck is not null && !IsSubtagActive())
|
||||
{
|
||||
// Preferred path: AVEVA's modern AlarmClient.AlarmAckByGUID carries
|
||||
// operator domain + full name and is not E_NOTIMPL like the wnwrap
|
||||
// COM AlarmAckByGUID.
|
||||
// COM AlarmAckByGUID. Skipped in subtag mode: those GUIDs are
|
||||
// synthetic and only the subtag consumer can resolve them (it acks
|
||||
// via an AckMsg write).
|
||||
return managedAck.AcknowledgeByGuid(
|
||||
alarmGuid,
|
||||
ackComment,
|
||||
|
||||
@@ -27,10 +27,10 @@ namespace ZB.MOM.WW.MxGateway.Worker.MxAccess;
|
||||
/// <see cref="WnalUserRequest"/>, <see cref="WnalUserRequestEx"/>) and the
|
||||
/// request-record layout are confirmed by disassembly (see
|
||||
/// docs/alarm-ack-findings.md in the reverse-engineering workspace). The
|
||||
/// list-ACQUISITION exports in <see cref="Unverified"/> are declared from
|
||||
/// their export names/ordinals but their argument signatures are NOT yet
|
||||
/// reverse-engineered — DO NOT call them until the signatures are verified,
|
||||
/// or the stdcall stack will be corrupted.
|
||||
/// list-ACQUISITION side (obtaining <c>hAlarmList</c> + alarm handles) is
|
||||
/// deliberately NOT implemented — see the note at the end of this class.
|
||||
/// Nothing in the worker calls this type yet; it is kept for when a real
|
||||
/// list handle can be supplied by a component that owns acquisition.
|
||||
/// </summary>
|
||||
public sealed class WnalNativeAckConsumer
|
||||
{
|
||||
@@ -101,7 +101,7 @@ public sealed class WnalNativeAckConsumer
|
||||
/// Build a request record and submit an ack for the given alarm
|
||||
/// handles via <see cref="WnalUserRequestEx"/>. Caller supplies a live
|
||||
/// <paramref name="hAlarmList"/> and the handle(s) obtained from the
|
||||
/// list's records (see <see cref="Unverified"/> — acquisition path).
|
||||
/// list's records (acquisition is not implemented — see the class note).
|
||||
/// Returns the native status (0 == success).
|
||||
/// </summary>
|
||||
/// <param name="hAlarmList">A registered, subscribed alarm-list handle.</param>
|
||||
|
||||
Reference in New Issue
Block a user