diff --git a/code-reviews/Driver.Cli.Common/findings.md b/code-reviews/Driver.Cli.Common/findings.md index cf991e2..356cc5f 100644 --- a/code-reviews/Driver.Cli.Common/findings.md +++ b/code-reviews/Driver.Cli.Common/findings.md @@ -105,7 +105,7 @@ on the masked code (`code & 0xFFFF0000`). | Severity | Medium | | Category | Concurrency & thread safety | | Location | `src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs:51-59` | -| Status | Open | +| Status | Resolved | **Description:** `ConfigureLogging` assigns the process-global `Serilog.Log.Logger` without disposing the previously assigned logger and the library never calls @@ -121,7 +121,7 @@ command `ExecuteAsync`, or via a `protected` disposal helper on this base). Trea `ConfigureLogging` as call-once / idempotent and document that. At minimum capture and dispose the previous logger if reconfiguration is genuinely intended. -**Resolution:** _(open)_ +**Resolution:** Resolved 2026-05-22 — `ConfigureLogging` is now idempotent (guarded by `_loggingConfigured` field) and disposes the previous `Log.Logger` before overwriting; a new `protected static FlushLogging()` helper calls `Log.CloseAndFlush()` for commands to call in their `finally` blocks; XML doc updated accordingly. ### Driver.Cli.Common-004 diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs index a745322..68500f5 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.Cli.Common/DriverCommandBase.cs @@ -7,8 +7,8 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Cli.Common; /// /// Shared base for every driver test-client command (Modbus / AB CIP / AB Legacy / S7 / -/// TwinCAT). Carries the options that are meaningful regardless of protocol — verbose -/// logging + the standard timeout — plus helpers every command implementation wants: +/// TwinCAT / FOCAS). Carries the options that are meaningful regardless of protocol — +/// verbose logging + the standard timeout — plus helpers every command implementation wants: /// Serilog configuration + cancellation-token capture. /// /// @@ -44,17 +44,37 @@ public abstract class DriverCommandBase : ICommand public abstract ValueTask ExecuteAsync(IConsole console); /// - /// Configures the process-global Serilog logger. Commands call this at the top of - /// so driver-internal Log.Logger writes land on the - /// same sink as the CLI's operator-facing output. + /// Configures the process-global Serilog logger. Intended to be called exactly once, + /// at the top of , so driver-internal Log.Logger + /// writes land on the same sink as the CLI's operator-facing output. + /// If the logger has already been configured this call is a no-op (idempotent). + /// Call in a finally block to ensure buffered output + /// is flushed before the process exits. /// protected void ConfigureLogging() { + if (_loggingConfigured) return; + _loggingConfigured = true; + + // Dispose the previous global logger (e.g. Serilog's silent bootstrap logger) so + // its resources are released cleanly before we overwrite Log.Logger. + var previous = Log.Logger; var config = new LoggerConfiguration(); if (Verbose) config.MinimumLevel.Debug().WriteTo.Console(); else config.MinimumLevel.Warning().WriteTo.Console(); Log.Logger = config.CreateLogger(); + (previous as IDisposable)?.Dispose(); } + + /// + /// Flushes and closes the Serilog logger configured by . + /// Call this in a finally block inside to prevent + /// buffered log output from being lost on process exit, particularly for long-running + /// commands such as subscribe. + /// + protected static void FlushLogging() => Log.CloseAndFlush(); + + private bool _loggingConfigured; }