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
|
`AcknowledgeAlarmByName` when the public RPC supplies a recognizable
|
||||||
`Provider!Group.Tag` reference.
|
`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
|
### 5. STA / threading — resolved
|
||||||
|
|
||||||
The wnwrap COM is `ThreadingModel=Apartment`. The consumer's
|
The wnwrap COM is `ThreadingModel=Apartment`. The consumer's
|
||||||
|
|||||||
@@ -446,6 +446,13 @@ behavior.
|
|||||||
The alarm monitor is independent of client sessions: `AcknowledgeAlarm` and
|
The alarm monitor is independent of client sessions: `AcknowledgeAlarm` and
|
||||||
`StreamAlarms` are session-less RPCs served by the monitor.
|
`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
|
### Alarm fallback options
|
||||||
|
|
||||||
The `Fallback` sub-section controls how the alarm feed selects between the
|
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);
|
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>
|
/// <summary>Verifies that acknowledge-by-name calls are forwarded to the consumer.</summary>
|
||||||
[Fact]
|
[Fact]
|
||||||
public void AcknowledgeByName_WhenInvoked_ForwardsToConsumerWithFullTuple()
|
public void AcknowledgeByName_WhenInvoked_ForwardsToConsumerWithFullTuple()
|
||||||
@@ -470,9 +564,13 @@ public sealed class AlarmDispatcherTests
|
|||||||
AlarmTransitionEmitted?.Invoke(this, transition);
|
AlarmTransitionEmitted?.Invoke(this, transition);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>Gets or sets a value indicating whether <see cref="Subscribe"/> throws.</summary>
|
||||||
|
public bool ThrowOnSubscribe { get; set; }
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
public void Subscribe(string subscription)
|
public void Subscribe(string subscription)
|
||||||
{
|
{
|
||||||
|
if (ThrowOnSubscribe) throw new InvalidOperationException("subscribe failed");
|
||||||
LastSubscription = subscription;
|
LastSubscription = subscription;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -533,4 +631,50 @@ public sealed class AlarmDispatcherTests
|
|||||||
Disposed = true;
|
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.
|
// 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);
|
MxAccessAlarmEventSink sink = new MxAccessAlarmEventSink(eventQueue, mapper);
|
||||||
dispatcher = new AlarmDispatcher(consumer, sink, subscribeSessionId, managedAck);
|
dispatcher = new AlarmDispatcher(consumer, sink, subscribeSessionId, managedAck);
|
||||||
try
|
try
|
||||||
|
|||||||
@@ -36,7 +36,7 @@ public sealed class AlarmDispatcher : IDisposable
|
|||||||
private readonly MxAccessAlarmEventSink sink;
|
private readonly MxAccessAlarmEventSink sink;
|
||||||
private readonly string sessionId;
|
private readonly string sessionId;
|
||||||
private readonly EventHandler<MxAlarmTransitionEvent> handler;
|
private readonly EventHandler<MxAlarmTransitionEvent> handler;
|
||||||
private readonly IManagedAlarmAckClient? managedAck;
|
private IManagedAlarmAckClient? managedAck;
|
||||||
private bool disposed;
|
private bool disposed;
|
||||||
|
|
||||||
/// <summary>Initializes a new alarm dispatcher for the given consumer, sink, and session ID.</summary>
|
/// <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">
|
/// <param name="managedAck">
|
||||||
/// Optional managed ack client. When non-null, GUID-keyed acknowledges
|
/// Optional managed ack client. When non-null, GUID-keyed acknowledges
|
||||||
/// route to it (AVEVA's modern <c>AlarmClient.AlarmAckByGUID</c>) instead
|
/// 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>
|
/// </param>
|
||||||
public AlarmDispatcher(
|
public AlarmDispatcher(
|
||||||
IMxAccessAlarmConsumer consumer,
|
IMxAccessAlarmConsumer consumer,
|
||||||
@@ -80,9 +82,30 @@ public sealed class AlarmDispatcher : IDisposable
|
|||||||
consumer.Subscribe(subscription);
|
consumer.Subscribe(subscription);
|
||||||
// The managed ack client maintains its own registered/subscribed
|
// The managed ack client maintains its own registered/subscribed
|
||||||
// consumer; run its lifecycle against the same expression so a later
|
// consumer; run its lifecycle against the same expression so a later
|
||||||
// AlarmAckByGUID has a live consumer to act through.
|
// AlarmAckByGUID has a live consumer to act through. It is an opt-in
|
||||||
managedAck?.Subscribe(subscription);
|
// 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>
|
/// <summary>
|
||||||
/// Forward an <c>AcknowledgeAlarm</c> request to the underlying
|
/// Forward an <c>AcknowledgeAlarm</c> request to the underlying
|
||||||
@@ -105,11 +128,13 @@ public sealed class AlarmDispatcher : IDisposable
|
|||||||
string ackOperatorFullName)
|
string ackOperatorFullName)
|
||||||
{
|
{
|
||||||
if (disposed) throw new ObjectDisposedException(nameof(AlarmDispatcher));
|
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
|
// Preferred path: AVEVA's modern AlarmClient.AlarmAckByGUID carries
|
||||||
// operator domain + full name and is not E_NOTIMPL like the wnwrap
|
// 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(
|
return managedAck.AcknowledgeByGuid(
|
||||||
alarmGuid,
|
alarmGuid,
|
||||||
ackComment,
|
ackComment,
|
||||||
|
|||||||
@@ -27,10 +27,10 @@ namespace ZB.MOM.WW.MxGateway.Worker.MxAccess;
|
|||||||
/// <see cref="WnalUserRequest"/>, <see cref="WnalUserRequestEx"/>) and the
|
/// <see cref="WnalUserRequest"/>, <see cref="WnalUserRequestEx"/>) and the
|
||||||
/// request-record layout are confirmed by disassembly (see
|
/// request-record layout are confirmed by disassembly (see
|
||||||
/// docs/alarm-ack-findings.md in the reverse-engineering workspace). The
|
/// docs/alarm-ack-findings.md in the reverse-engineering workspace). The
|
||||||
/// list-ACQUISITION exports in <see cref="Unverified"/> are declared from
|
/// list-ACQUISITION side (obtaining <c>hAlarmList</c> + alarm handles) is
|
||||||
/// their export names/ordinals but their argument signatures are NOT yet
|
/// deliberately NOT implemented — see the note at the end of this class.
|
||||||
/// reverse-engineered — DO NOT call them until the signatures are verified,
|
/// Nothing in the worker calls this type yet; it is kept for when a real
|
||||||
/// or the stdcall stack will be corrupted.
|
/// list handle can be supplied by a component that owns acquisition.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public sealed class WnalNativeAckConsumer
|
public sealed class WnalNativeAckConsumer
|
||||||
{
|
{
|
||||||
@@ -101,7 +101,7 @@ public sealed class WnalNativeAckConsumer
|
|||||||
/// Build a request record and submit an ack for the given alarm
|
/// Build a request record and submit an ack for the given alarm
|
||||||
/// handles via <see cref="WnalUserRequestEx"/>. Caller supplies a live
|
/// handles via <see cref="WnalUserRequestEx"/>. Caller supplies a live
|
||||||
/// <paramref name="hAlarmList"/> and the handle(s) obtained from the
|
/// <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).
|
/// Returns the native status (0 == success).
|
||||||
/// </summary>
|
/// </summary>
|
||||||
/// <param name="hAlarmList">A registered, subscribed alarm-list handle.</param>
|
/// <param name="hAlarmList">A registered, subscribed alarm-list handle.</param>
|
||||||
|
|||||||
Reference in New Issue
Block a user