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

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:
Joseph Doherty
2026-10-18 13:11:01 -04:00
parent 6affbd57ea
commit 6f4c7c3bf1
6 changed files with 214 additions and 12 deletions
+22
View File
@@ -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
+7
View File
@@ -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>